diff --git a/client/client.go b/client/client.go index 76cce81e..40b86969 100644 --- a/client/client.go +++ b/client/client.go @@ -15,12 +15,13 @@ import ( ) var ( - ErrorInvalidClientID = errors.New("not a valid client ID") - ErrorInvalidRedirectURL = errors.New("not a valid redirect url for the given client") - ErrorCantChooseRedirectURL = errors.New("must provide a redirect url; client has many") - ErrorNoValidRedirectURLs = errors.New("no valid redirect URLs for this client.") - ErrorPublicClientRedirectURIs = errors.New("native clients cannot have redirect URIs") - ErrorPublicClientMissingName = errors.New("native clients must have a name") + ErrorInvalidClientID = errors.New("not a valid client ID") + ErrorInvalidRedirectURL = errors.New("not a valid redirect url for the given client") + ErrorCantChooseRedirectURL = errors.New("must provide a redirect url; client has many") + ErrorNoValidRedirectURLs = errors.New("no valid redirect URLs for this client.") + + ErrorPublicClientRedirectURIs = errors.New("public clients cannot have redirect URIs") + ErrorPublicClientMissingName = errors.New("public clients must have a name") ErrorMissingRedirectURI = errors.New("no client redirect url given") diff --git a/client/manager/manager.go b/client/manager/manager.go index 5ee0790b..fb5f51b0 100644 --- a/client/manager/manager.go +++ b/client/manager/manager.go @@ -2,6 +2,7 @@ package manager import ( "encoding/base64" + "net/url" "errors" @@ -21,6 +22,10 @@ const ( maxSecretLength = 72 ) +var ( + localHostRedirectURL = mustParseURL("http://localhost:0") +) + type ClientOptions struct { TrustedPeers []string } @@ -67,6 +72,10 @@ func NewClientManager(clientRepo client.ClientRepo, txnFactory repo.TransactionF } } +// New creates and persists a new client with the given options, returning the generated credentials. +// Any Credenials provided with the client are ignored and overwritten by the generated ID and Secret. +// "Normal" (i.e. non-Public) clients must have at least one valid RedirectURI in their Metadata. +// Public clients must not have any RedirectURIs and must have a client name. func (m *ClientManager) New(cli client.Client, options *ClientOptions) (*oidc.ClientCredentials, error) { tx, err := m.begin() if err != nil { @@ -74,11 +83,14 @@ func (m *ClientManager) New(cli client.Client, options *ClientOptions) (*oidc.Cl } defer tx.Rollback() + if err := validateClient(cli); err != nil { + return nil, err + } + err = m.addClientCredentials(&cli) if err != nil { return nil, err } - creds := cli.Credentials // Save Client @@ -171,11 +183,15 @@ func (m *ClientManager) Authenticate(creds oidc.ClientCredentials) (bool, error) } func (m *ClientManager) addClientCredentials(cli *client.Client) error { - // Generate Client ID - if len(cli.Metadata.RedirectURIs) < 1 { - return errors.New("no client redirect url given") + var seed string + if cli.Public { + seed = cli.Metadata.ClientName + } else { + seed = cli.Metadata.RedirectURIs[0].Host } - clientID, err := m.clientIDGenerator(cli.Metadata.RedirectURIs[0].Host) + + // Generate Client ID + clientID, err := m.clientIDGenerator(seed) if err != nil { return err } @@ -192,3 +208,37 @@ func (m *ClientManager) addClientCredentials(cli *client.Client) error { } return nil } + +func validateClient(cli client.Client) error { + // NOTE: please be careful changing the errors returned here; they are used + // downstream (eg. in the admin API) to determine the http errors returned. + if cli.Public { + if len(cli.Metadata.RedirectURIs) > 0 { + return client.ErrorPublicClientRedirectURIs + } + if cli.Metadata.ClientName == "" { + return client.ErrorPublicClientMissingName + } + cli.Metadata.RedirectURIs = []url.URL{ + localHostRedirectURL, + } + } else { + if len(cli.Metadata.RedirectURIs) < 1 { + return client.ErrorMissingRedirectURI + } + } + + err := cli.Metadata.Valid() + if err != nil { + return client.ValidationError{Err: err} + } + return nil +} + +func mustParseURL(s string) url.URL { + u, err := url.Parse(s) + if err != nil { + panic(err) + } + return *u +} diff --git a/client/manager/manager_test.go b/client/manager/manager_test.go index 1d489631..c6db2185 100644 --- a/client/manager/manager_test.go +++ b/client/manager/manager_test.go @@ -168,3 +168,53 @@ func TestAuthenticate(t *testing.T) { } } } + +func TestValidateClient(t *testing.T) { + tests := []struct { + cli client.Client + wantErr error + }{ + { + cli: client.Client{ + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{mustParseURL("http://auth.google.com")}, + }, + }, + }, + { + cli: client.Client{}, + wantErr: client.ErrorMissingRedirectURI, + }, + { + cli: client.Client{ + Metadata: oidc.ClientMetadata{ + ClientName: "frank", + }, + Public: true, + }, + }, + { + cli: client.Client{ + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{mustParseURL("http://auth.google.com")}, + ClientName: "frank", + }, + Public: true, + }, + wantErr: client.ErrorPublicClientRedirectURIs, + }, + { + cli: client.Client{ + Public: true, + }, + wantErr: client.ErrorPublicClientMissingName, + }, + } + + for i, tt := range tests { + err := validateClient(tt.cli) + if err != tt.wantErr { + t.Errorf("case %d: want=%v, got=%v", i, tt.wantErr, err) + } + } +}