Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for custom keyring names on linux #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 44 additions & 1 deletion keyring.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package keyring

import "fmt"
import (
"fmt"
)

// provider set in the init function by the relevant os file e.g.:
// keyring_linux.go
Expand Down Expand Up @@ -36,3 +38,44 @@ func Get(service, user string) (string, error) {
func Delete(service, user string) error {
return provider.Delete(service, user)
}

// CustomKeyring allows to use custom keyring names
type CustomKeyring struct {
keyring Keyring
keyringName string
}

// NewCustomKeyring create a new custom keyring.
// A custom keyring allows you to use a custom
// keyring name on supported platforms
func NewCustomKeyring(name string) *CustomKeyring {
ck := CustomKeyring{
keyringName: name,
}

// Set customs keyringprovider to current provider
ck.keyring = provider

// If provider is 'supported' set its keyring name
if pv, ok := provider.(secretServiceProvider); ok {
pv.keyringName = name
ck.keyring = pv
}

return &ck
}

// Set password in keyring for user.
func (ck *CustomKeyring) Set(service, user, password string) error {
return ck.keyring.Set(service, user, password)
}

// Get password from keyring given service and user name.
func (ck *CustomKeyring) Get(service, user string) (string, error) {
return ck.keyring.Get(service, user)
}

// Delete secret from keyring.
func (ck *CustomKeyring) Delete(service, user string) error {
return ck.keyring.Delete(service, user)
}
14 changes: 14 additions & 0 deletions keyring_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@ const (
encodingPrefix = "go-keyring-encoded:"
)

// ignore, just for typecast in keyring.go
type secretServiceProvider struct {
keyringName string
}

// Set password in keyring for user.
func (s secretServiceProvider) Set(service, user, password string) error { return nil }

// Get password from keyring given service and user name.
func (s secretServiceProvider) Get(service, user string) (string, error) { return "", nil }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not this return DefaultKeyringName?


// Delete secret from keyring.
func (s secretServiceProvider) Delete(service, user string) error { return nil }

type macOSXKeychain struct{}

// func (*MacOSXKeychain) IsAvailable() bool {
Expand Down
26 changes: 18 additions & 8 deletions keyring_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,20 @@ package keyring

import (
"fmt"

"github.com/godbus/dbus"
"github.com/zalando/go-keyring/secret_service"
ss "github.com/zalando/go-keyring/secret_service"
)

type secretServiceProvider struct{}
// secretServiceProvider linux provider
type secretServiceProvider struct {
keyringName string
}

// Set stores user and pass in the keyring under the defined service
// name.
func (s secretServiceProvider) Set(service, user, pass string) error {
svc, err := ss.NewSecretService()
svc, err := ss.NewSecretService(s.keyringName)
if err != nil {
return err
}
Expand All @@ -30,7 +34,10 @@ func (s secretServiceProvider) Set(service, user, pass string) error {

secret := ss.NewSecret(session.Path(), pass)

collection := svc.GetLoginCollection()
collection, err := svc.GetCollectionForKeyring()
if err != nil {
return err
}

err = svc.Unlock(collection.Path())
if err != nil {
Expand All @@ -49,14 +56,17 @@ func (s secretServiceProvider) Set(service, user, pass string) error {

// findItem looksup an item by service and user.
func (s secretServiceProvider) findItem(svc *ss.SecretService, service, user string) (dbus.ObjectPath, error) {
collection := svc.GetLoginCollection()
collection, err := svc.GetCollectionForKeyring()
if err != nil {
return "", err
}

search := map[string]string{
"username": user,
"service": service,
}

err := svc.Unlock(collection.Path())
err = svc.Unlock(collection.Path())
if err != nil {
return "", err
}
Expand All @@ -75,7 +85,7 @@ func (s secretServiceProvider) findItem(svc *ss.SecretService, service, user str

// Get gets a secret from the keyring given a service name and a user.
func (s secretServiceProvider) Get(service, user string) (string, error) {
svc, err := ss.NewSecretService()
svc, err := ss.NewSecretService(s.keyringName)
if err != nil {
return "", err
}
Expand All @@ -102,7 +112,7 @@ func (s secretServiceProvider) Get(service, user string) (string, error) {

// Delete deletes a secret, identified by service & user, from the keyring.
func (s secretServiceProvider) Delete(service, user string) error {
svc, err := ss.NewSecretService()
svc, err := ss.NewSecretService(s.keyringName)
if err != nil {
return err
}
Expand Down
35 changes: 34 additions & 1 deletion keyring_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package keyring

import "testing"
import (
"testing"
)

const (
service = "test-service"
Expand Down Expand Up @@ -96,3 +98,34 @@ func TestDeleteNonExisting(t *testing.T) {
t.Errorf("Expected error ErrNotFound, got %s", err)
}
}

// TestCustomKeyringSet tests deleting a secret from a custom keyring.
func TestCustomKeyringSet(t *testing.T) {
keyRing := NewCustomKeyring("_keyring_test")
err := keyRing.Set(service, user, password)
if err != nil {
t.Errorf("Should not fail: %s", err)
}
}

// TestCustomKeyringGet tests getting a password from a custom keyring.
func TestCustomKeyringGet(t *testing.T) {
keyRing := NewCustomKeyring("_keyring_test")
pass, err := keyRing.Get(service, user)
if err != nil {
t.Errorf("Should not fail: %s", err)
}

if pass != password {
t.Errorf("Expected password %s, got %s", password, pass)
}
}

// TestCustomKeyringDelete tests deleting a secret from a custom keyring.
func TestCustomKeyringDelete(t *testing.T) {
keyRing := NewCustomKeyring("_keyring_test")
err := keyRing.Delete(service, user)
if err != nil {
t.Errorf("Should not fail, got: %s", err)
}
}
17 changes: 16 additions & 1 deletion keyring_windows.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
package keyring

import (
"github.com/danieljoos/wincred"
"syscall"

"github.com/danieljoos/wincred"
)

// ignore, just for typecast in keyring.go
type secretServiceProvider struct {
keyringName string
}

// Set password in keyring for user.
func (s secretServiceProvider) Set(service, user, password string) error { return nil }

// Get password from keyring given service and user name.
func (s secretServiceProvider) Get(service, user string) (string, error) { return "", nil }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not this return DefaultKeyringName?

Copy link
Author

@JojiiOfficial JojiiOfficial Apr 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those functions aren't getting called at all. Those are just for the compiler to not throw an error. Otherwise on any other GOOS than linux the compiler would say that there is no such secretServiceProvider to typecast to and it wouldn't compile.
Same in keyring_darwin.go

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it looks like you should refactor this part, such that the empty string check should be dropped and the default should be returned by the new functions created.

@mikkeloscar Wdyt?


// Delete secret from keyring.
func (s secretServiceProvider) Delete(service, user string) error { return nil }

type windowsKeychain struct{}

// Get gets a secret from the keyring given a service name and a user.
Expand Down
56 changes: 51 additions & 5 deletions secret_service/secret_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package ss

import (
"fmt"
"regexp"

"errors"

"github.com/godbus/dbus"
)

Expand All @@ -21,6 +23,11 @@ const (
collectionBasePath = "/org/freedesktop/secrets/collection/"
)

const (
// DefaultKeyringName the name of the keyring to use as default
DefaultKeyringName = "login"
)

// Secret defines a org.freedesk.Secret.Item secret struct.
type Secret struct {
Session dbus.ObjectPath
Expand All @@ -42,22 +49,31 @@ func NewSecret(session dbus.ObjectPath, secret string) Secret {
// SecretService is an interface for the Secret Service dbus API.
type SecretService struct {
*dbus.Conn
object dbus.BusObject
object dbus.BusObject
KeyringName string
}

// NewSecretService inializes a new SecretService object.
func NewSecretService() (*SecretService, error) {
func NewSecretService(keyringName string) (*SecretService, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A breaking change in libraries is not a great thing to do.
Please let the signature like this and add another func:

func WithKeyringName(name string) (*SecretService, error) {
    s,err := NewSecretService()
    if err != nil { return nil, err }
    s.KeyringName = name
    return s, nil
}

conn, err := dbus.SessionBus()
if err != nil {
return nil, err
}

return &SecretService{
conn,
conn.Object(serviceName, servicePath),
Conn: conn,
object: conn.Object(serviceName, servicePath),
KeyringName: formatKeyringName(keyringName),
}, nil
}

// see https://lists.freedesktop.org/archives/systemd-devel/2013-March/009402.html
func formatKeyringName(name string) string {
re := regexp.MustCompile("[^A-Za-z0-9]")
name = re.ReplaceAllString(name, "_5f")
return name
}

// OpenSession opens a secret service session.
func (s *SecretService) OpenSession() (dbus.BusObject, error) {
var disregard dbus.Variant
Expand Down Expand Up @@ -87,9 +103,39 @@ func (s *SecretService) CheckCollectionPath(path dbus.ObjectPath) error {
return errors.New("path not found")
}

// GetCollectionForKeyring returns collection from SecretService
func (s *SecretService) GetCollectionForKeyring() (dbus.BusObject, error) {
var collection dbus.BusObject
// Pick requested collection
if len(s.KeyringName) == 0 || s.KeyringName == DefaultKeyringName {
collection = s.GetLoginCollection()
} else {
// Check for customs collections availability
collectionPath := s.GetCollectionPath(s.KeyringName)
err := s.CheckCollectionPath(collectionPath)
if err != nil {
return nil, err
}

// If available
collection = s.GetCollectionByPath(collectionPath)
}
return collection, nil
}

// GetCollectionPath get path of collection by its name
func (s *SecretService) GetCollectionPath(name string) dbus.ObjectPath {
return dbus.ObjectPath(collectionBasePath + name)
}

// GetCollection returns a collection from a name.
func (s *SecretService) GetCollection(name string) dbus.BusObject {
return s.Object(serviceName, dbus.ObjectPath(collectionBasePath+name))
return s.GetCollectionByPath(s.GetCollectionPath(name))
}

// GetCollectionByPath returns a collection from a name.
func (s *SecretService) GetCollectionByPath(path dbus.ObjectPath) dbus.BusObject {
return s.Object(serviceName, path)
}

// GetLoginCollection decides and returns the dbus collection to be used for login.
Expand Down