From 82a55cf785a6307b8ee552e9f2a19468882acfd3 Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Wed, 14 Sep 2016 16:38:12 -0700 Subject: [PATCH] {server,storage}: add LoggedIn flag to AuthRequest and improve storage docs Currently, whether or not a user has authenticated themselves through a connector is indicated by a pointer being nil or non-nil. Instead add an explicit flag that marks this. --- server/handlers.go | 9 ++-- storage/kubernetes/types.go | 16 +++--- storage/storage.go | 80 +++++++++++++++++++++++------- storage/storagetest/storagetest.go | 9 ++-- 4 files changed, 76 insertions(+), 38 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index ebbc8740..ef3e9af5 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -264,7 +264,8 @@ func (s *Server) finalizeLogin(identity connector.Identity, authReqID, connector } updater := func(a storage.AuthRequest) (storage.AuthRequest, error) { - a.Claims = &claims + a.LoggedIn = true + a.Claims = claims a.ConnectorID = connectorID a.ConnectorData = identity.ConnectorData return a, nil @@ -282,7 +283,7 @@ func (s *Server) handleApproval(w http.ResponseWriter, r *http.Request) { s.renderError(w, http.StatusInternalServerError, errServerError, "") return } - if authReq.Claims == nil { + if !authReq.LoggedIn { log.Printf("Auth request does not have an identity for approval") s.renderError(w, http.StatusInternalServerError, errServerError, "") return @@ -341,7 +342,7 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe ConnectorID: authReq.ConnectorID, Nonce: authReq.Nonce, Scopes: authReq.Scopes, - Claims: *authReq.Claims, + Claims: authReq.Claims, Expiry: s.now().Add(time.Minute * 5), RedirectURI: authReq.RedirectURI, } @@ -358,7 +359,7 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe } q.Set("code", code.ID) case responseTypeToken: - idToken, expiry, err := s.newIDToken(authReq.ClientID, *authReq.Claims, authReq.Scopes, authReq.Nonce) + idToken, expiry, err := s.newIDToken(authReq.ClientID, authReq.Claims, authReq.Scopes, authReq.Nonce) if err != nil { log.Printf("failed to create ID token: %v", err) tokenErr(w, errServerError, "", http.StatusInternalServerError) diff --git a/storage/kubernetes/types.go b/storage/kubernetes/types.go index 1440057d..8bc934f2 100644 --- a/storage/kubernetes/types.go +++ b/storage/kubernetes/types.go @@ -118,9 +118,11 @@ type AuthRequest struct { // attempts. ForceApprovalPrompt bool `json:"forceApprovalPrompt,omitempty"` + LoggedIn bool `json:"loggedIn"` + // The identity of the end user. Generally nil until the user authenticates // with a backend. - Claims *Claims `json:"claims,omitempty"` + Claims Claims `json:"claims,omitempty"` // The connector used to login the user. Set when the user authenticates. ConnectorID string `json:"connectorID,omitempty"` ConnectorData []byte `json:"connectorData,omitempty"` @@ -145,13 +147,11 @@ func toStorageAuthRequest(req AuthRequest) storage.AuthRequest { Nonce: req.Nonce, State: req.State, ForceApprovalPrompt: req.ForceApprovalPrompt, + LoggedIn: req.LoggedIn, ConnectorID: req.ConnectorID, ConnectorData: req.ConnectorData, Expiry: req.Expiry, - } - if req.Claims != nil { - i := toStorageClaims(*req.Claims) - a.Claims = &i + Claims: toStorageClaims(req.Claims), } return a } @@ -172,14 +172,12 @@ func (cli *client) fromStorageAuthRequest(a storage.AuthRequest) AuthRequest { RedirectURI: a.RedirectURI, Nonce: a.Nonce, State: a.State, + LoggedIn: a.LoggedIn, ForceApprovalPrompt: a.ForceApprovalPrompt, ConnectorID: a.ConnectorID, ConnectorData: a.ConnectorData, Expiry: a.Expiry, - } - if a.Claims != nil { - i := fromStorageClaims(*a.Claims) - req.Claims = &i + Claims: fromStorageClaims(a.Claims), } return req } diff --git a/storage/storage.go b/storage/storage.go index 399b35cd..da441b19 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -70,28 +70,41 @@ type Storage interface { DeleteRefresh(id string) error // Update functions are assumed to be a performed within a single object transaction. + // + // updaters may be called multiple times. UpdateClient(id string, updater func(old Client) (Client, error)) error UpdateKeys(updater func(old Keys) (Keys, error)) error UpdateAuthRequest(id string, updater func(a AuthRequest) (AuthRequest, error)) error + + // TODO(ericchiang): Add a GarbageCollect(now time.Time) method so conformance tests + // can test implementations. } -// Client is an OAuth2 client. +// Client represents an OAuth2 client. // // For further reading see: // * Trusted peers: https://developers.google.com/identity/protocols/CrossClientAuth // * Public clients: https://developers.google.com/api-client-library/python/auth/installed-app type Client struct { - ID string `json:"id" yaml:"id"` - Secret string `json:"secret" yaml:"secret"` + // Client ID and secret used to identify the client. + ID string `json:"id" yaml:"id"` + Secret string `json:"secret" yaml:"secret"` + + // A registered set of redirect URIs. When redirecting from dex to the client, the URI + // requested to redirect to MUST match one of these values, unless the client is "public". RedirectURIs []string `json:"redirectURIs" yaml:"redirectURIs"` - // TrustedPeers are a list of peers which can issue tokens on this client's behalf. + // TrustedPeers are a list of peers which can issue tokens on this client's behalf using + // the dynamic "oauth2:server:client_id:(client_id)" scope. If a peer makes such a request, + // this client's ID will appear as the ID Token's audience. + // // Clients inherently trust themselves. TrustedPeers []string `json:"trustedPeers" yaml:"trustedPeers"` // Public clients must use either use a redirectURL 127.0.0.1:X or "urn:ietf:wg:oauth:2.0:oob" Public bool `json:"public" yaml:"public"` + // Name and LogoURL used when displaying this client to the end user. Name string `json:"name" yaml:"name"` LogoURL string `json:"logoURL" yaml:"logoURL"` } @@ -109,53 +122,79 @@ type Claims struct { // AuthRequest represents a OAuth2 client authorization request. It holds the state // of a single auth flow up to the point that the user authorizes the client. type AuthRequest struct { - ID string + // ID used to identify the authorization request. + ID string + + // ID of the client requesting authorization from a user. ClientID string + // Values parsed from the initial request. These describe the resources the client is + // requesting as well as values describing the form of the response. ResponseTypes []string Scopes []string RedirectURI string - - Nonce string - State string + Nonce string + State string // The client has indicated that the end user must be shown an approval prompt // on all requests. The server cannot cache their initial action for subsequent // attempts. ForceApprovalPrompt bool + Expiry time.Time + + // Has the user proved their identity through a backing identity provider? + // + // If false, the following fields are invalid. + LoggedIn bool + // The identity of the end user. Generally nil until the user authenticates // with a backend. - Claims *Claims + Claims Claims // The connector used to login the user and any data the connector wishes to persists. // Set when the user authenticates. ConnectorID string ConnectorData []byte - - Expiry time.Time } // AuthCode represents a code which can be exchanged for an OAuth2 token response. +// +// This value is created once an end user has authorized a client, the server has +// redirect the end user back to the client, but the client hasn't exchanged the +// code for an access_token and id_token. type AuthCode struct { + // Actual string returned as the "code" value. ID string - ClientID string + // The client this code value is valid for. When exchanging the code for a + // token response, the client must use its client_secret to authenticate. + ClientID string + + // As part of the OAuth2 spec when a client makes a token request it MUST + // present the same redirect_uri as the initial redirect. This values is saved + // to make this check. + // + // https://tools.ietf.org/html/rfc6749#section-4.1.3 RedirectURI string - ConnectorID string - ConnectorData []byte - + // If provided by the client in the initial request, the provider MUST create + // a ID Token with this nonce in the JWT payload. Nonce string + // Scopes authorized by the end user for the client. Scopes []string - Claims Claims + // Authentication data provided by an upstream source. + ConnectorID string + ConnectorData []byte + Claims Claims Expiry time.Time } -// RefreshToken is an OAuth2 refresh token. +// RefreshToken is an OAuth2 refresh token which allows a client to request new +// tokens on the end user's behalf. type RefreshToken struct { // The actual refresh token. RefreshToken string @@ -163,17 +202,19 @@ type RefreshToken struct { // Client this refresh token is valid for. ClientID string + // Authentication data provided by an upstream source. ConnectorID string ConnectorData []byte + Claims Claims // Scopes present in the initial request. Refresh requests may specify a set // of scopes different from the initial request when refreshing a token, // however those scopes must be encompassed by this set. Scopes []string + // Nonce value supplied during the initial redirect. This is required to be part + // of the claims of any future id_token generated by the client. Nonce string - - Claims Claims } // VerificationKey is a rotated signing key which can still be used to verify @@ -188,6 +229,7 @@ type Keys struct { // Key for creating and verifying signatures. These may be nil. SigningKey *jose.JSONWebKey SigningKeyPub *jose.JSONWebKey + // Old signing keys which have been rotated but can still be used to validate // existing signatures. VerificationKeys []VerificationKey diff --git a/storage/storagetest/storagetest.go b/storage/storagetest/storagetest.go index c4f8f96e..e0dac52e 100644 --- a/storage/storagetest/storagetest.go +++ b/storage/storagetest/storagetest.go @@ -35,7 +35,7 @@ func testUpdateAuthRequest(t *testing.T, s storage.Storage) { t.Fatalf("failed creating auth request: %v", err) } if err := s.UpdateAuthRequest(a.ID, func(old storage.AuthRequest) (storage.AuthRequest, error) { - old.Claims = &identity + old.Claims = identity old.ConnectorID = "connID" return old, nil }); err != nil { @@ -46,11 +46,8 @@ func testUpdateAuthRequest(t *testing.T, s storage.Storage) { if err != nil { t.Fatalf("failed to get auth req: %v", err) } - if got.Claims == nil { - t.Fatalf("no identity in auth request") - } - if !reflect.DeepEqual(*got.Claims, identity) { - t.Fatalf("update failed, wanted identity=%#v got %#v", identity, *got.Claims) + if !reflect.DeepEqual(got.Claims, identity) { + t.Fatalf("update failed, wanted identity=%#v got %#v", identity, got.Claims) } }