diff --git a/server/user.go b/server/user.go index bf870701..d4901a48 100644 --- a/server/user.go +++ b/server/user.go @@ -2,7 +2,6 @@ package server import ( "encoding/json" - "errors" "net/http" "net/url" "strconv" @@ -262,18 +261,32 @@ func (s *UserMgmtServer) getCreds(r *http.Request, requiresAdmin bool) (api.Cred return api.Creds{}, api.ErrorUnauthorized } - clientID, ok, err := claims.StringClaim("aud") - if err != nil { - log.Errorf("userMgmtServer: GetCreds err: %q", err) - return api.Creds{}, err + // The "aud" claim is allowed to be both a list of clients or a single client. Check for both cases. + clientIDs, ok, err := claims.StringsClaim("aud") + if err != nil || !ok { + clientID, ok, err := claims.StringClaim("aud") + if err != nil { + log.Errorf("userMgmtServer: GetCreds failed to parse 'aud' claim: %q", err) + return api.Creds{}, api.ErrorUnauthorized + } + if !ok || clientID == "" { + return api.Creds{}, api.ErrorUnauthorized + } + clientIDs = []string{clientID} } - if !ok || clientID == "" { - return api.Creds{}, errors.New("no aud(client ID) claim") + if len(clientIDs) == 0 { + log.Errorf("userMgmtServer: GetCreds err: no client in audience") + return api.Creds{}, api.ErrorUnauthorized } - verifier := s.jwtvFactory(clientID) + // Verify that the JWT is signed by this server, has the correct issuer, hasn't expired, etc. + // While we don't actualy care which client the token was issued for (we'll check that later), + // go-oidc doesn't provide any methods which don't require passing a client ID. + // + // TODO(ericchiang): Add a verifier to go-oidc that doesn't require a client ID. + verifier := s.jwtvFactory(clientIDs[0]) if err := verifier.Verify(jwt); err != nil { - log.Errorf("userMgmtServer: GetCreds err: %q", err) + log.Errorf("userMgmtServer: GetCreds err: failed to verify token %q", err) return api.Creds{}, api.ErrorUnauthorized } @@ -295,18 +308,32 @@ func (s *UserMgmtServer) getCreds(r *http.Request, requiresAdmin bool) (api.Cred return api.Creds{}, err } - isAdmin, err := s.cm.IsDexAdmin(clientID) - if err != nil { - log.Errorf("userMgmtServer: GetCreds err: %q", err) - return api.Creds{}, err + i := 0 + for _, clientID := range clientIDs { + // Make sure the client actually exists. + isAdmin, err := s.cm.IsDexAdmin(clientID) + if err != nil { + log.Errorf("userMgmtServer: GetCreds err: failed to get client %v", err) + return api.Creds{}, err + } + + // If the endpoint requires an admin client, filter out clients which are not admins. + if requiresAdmin && !isAdmin { + continue + } + + clientIDs[i] = clientID + i++ } - if requiresAdmin && !isAdmin { + + clientIDs = clientIDs[:i] + if len(clientIDs) == 0 { return api.Creds{}, api.ErrorForbidden } return api.Creds{ - ClientID: clientID, - User: usr, + ClientIDs: clientIDs, + User: usr, }, nil } diff --git a/user/api/api.go b/user/api/api.go index 148647fb..64aeab0e 100644 --- a/user/api/api.go +++ b/user/api/api.go @@ -98,8 +98,9 @@ type Emailer interface { } type Creds struct { - ClientID string - User user.User + // IDTokens can be issued for multiple clients. + ClientIDs []string + User user.User } // TODO(ericchiang): Don't pass a dbMap. See #385. @@ -144,6 +145,22 @@ func (u *UsersAPI) DisableUser(creds Creds, userID string, disable bool) (schema }, nil } +// validRedirectURL finds the first client for which the redirect URL is valid. If found it returns the client_id of the client. +func validRedirectURL(clientManager *clientmanager.ClientManager, redirectURL url.URL, clientIDs []string) (string, error) { + // Find the first client with a valid redirectURL. + for _, clientID := range clientIDs { + metadata, err := clientManager.Metadata(clientID) + if err != nil { + return "", mapError(err) + } + + if _, err := client.ValidRedirectURL(&redirectURL, metadata.RedirectURIs); err == nil { + return clientID, nil + } + } + return "", ErrorInvalidRedirectURL +} + func (u *UsersAPI) CreateUser(creds Creds, usr schema.User, redirURL url.URL) (schema.UserCreateResponse, error) { log.Infof("userAPI: CreateUser") if !u.Authorize(creds) { @@ -155,14 +172,9 @@ func (u *UsersAPI) CreateUser(creds Creds, usr schema.User, redirURL url.URL) (s return schema.UserCreateResponse{}, mapError(err) } - metadata, err := u.clientManager.Metadata(creds.ClientID) + clientID, err := validRedirectURL(u.clientManager, redirURL, creds.ClientIDs) if err != nil { - return schema.UserCreateResponse{}, mapError(err) - } - - validRedirURL, err := client.ValidRedirectURL(&redirURL, metadata.RedirectURIs) - if err != nil { - return schema.UserCreateResponse{}, ErrorInvalidRedirectURL + return schema.UserCreateResponse{}, err } id, err := u.userManager.CreateUser(schemaUserToUser(usr), user.Password(hash), u.localConnectorID) @@ -177,7 +189,7 @@ func (u *UsersAPI) CreateUser(creds Creds, usr schema.User, redirURL url.URL) (s usr = userToSchemaUser(userUser) - url, err := u.emailer.SendInviteEmail(usr.Email, validRedirURL, creds.ClientID) + url, err := u.emailer.SendInviteEmail(usr.Email, redirURL, clientID) // An email is sent only if we don't get a link and there's no error. emailSent := err == nil && url == nil @@ -200,14 +212,9 @@ func (u *UsersAPI) ResendEmailInvitation(creds Creds, userID string, redirURL ur return schema.ResendEmailInvitationResponse{}, ErrorUnauthorized } - metadata, err := u.clientManager.Metadata(creds.ClientID) + clientID, err := validRedirectURL(u.clientManager, redirURL, creds.ClientIDs) if err != nil { - return schema.ResendEmailInvitationResponse{}, mapError(err) - } - - validRedirURL, err := client.ValidRedirectURL(&redirURL, metadata.RedirectURIs) - if err != nil { - return schema.ResendEmailInvitationResponse{}, ErrorInvalidRedirectURL + return schema.ResendEmailInvitationResponse{}, err } // Retrieve user to check if it's already created @@ -221,7 +228,7 @@ func (u *UsersAPI) ResendEmailInvitation(creds Creds, userID string, redirURL ur return schema.ResendEmailInvitationResponse{}, ErrorVerifiedEmail } - url, err := u.emailer.SendInviteEmail(userUser.Email, validRedirURL, creds.ClientID) + url, err := u.emailer.SendInviteEmail(userUser.Email, redirURL, clientID) // An email is sent only if we don't get a link and there's no error. emailSent := err == nil && url == nil diff --git a/user/api/api_test.go b/user/api/api_test.go index 12a1f64e..81624e58 100644 --- a/user/api/api_test.go +++ b/user/api/api_test.go @@ -51,15 +51,16 @@ func (t *testEmailer) sendEmail(email string, redirectURL url.URL, clientID stri } var ( - clock = clockwork.NewFakeClock() - goodClientID = "client.example.com" + clock = clockwork.NewFakeClock() + goodClientID = "client.example.com" + nonAdminClientID = "user.example.com" goodCreds = Creds{ User: user.User{ ID: "ID-1", Admin: true, }, - ClientID: goodClientID, + ClientIDs: []string{goodClientID}, } badCreds = Creds{ @@ -68,13 +69,21 @@ var ( }, } + credsWithMultipleAudiences = Creds{ + User: user.User{ + ID: "ID-1", + Admin: true, + }, + ClientIDs: []string{nonAdminClientID, goodClientID}, + } + disabledCreds = Creds{ User: user.User{ ID: "ID-1", Admin: true, Disabled: true, }, - ClientID: goodClientID, + ClientIDs: []string{goodClientID}, } resetPasswordURL = url.URL{ @@ -87,6 +96,11 @@ var ( Host: goodClientID, Path: "/callback", } + validRedirURL2 = url.URL{ + Scheme: "http", + Host: nonAdminClientID, + Path: "/callback", + } ) func makeTestFixtures() (*UsersAPI, *testEmailer) { @@ -169,6 +183,17 @@ func makeTestFixtures() (*UsersAPI, *testEmailer) { }, }, } + ci2 := client.Client{ + Credentials: oidc.ClientCredentials{ + ID: nonAdminClientID, + Secret: base64.URLEncoding.EncodeToString([]byte("anothersecret")), + }, + Metadata: oidc.ClientMetadata{ + RedirectURIs: []url.URL{ + validRedirURL2, + }, + }, + } clientIDGenerator := func(hostport string) (string, error) { return hostport, nil @@ -176,7 +201,7 @@ func makeTestFixtures() (*UsersAPI, *testEmailer) { secGen := func() ([]byte, error) { return []byte("secret"), nil } - clientRepo, err := db.NewClientRepoFromClients(dbMap, []client.LoadableClient{{Client: ci}}) + clientRepo, err := db.NewClientRepoFromClients(dbMap, []client.LoadableClient{{Client: ci}, {Client: ci2}}) if err != nil { panic("Failed to create client manager: " + err.Error()) } @@ -223,6 +248,10 @@ func TestGetUser(t *testing.T) { id: "NO_ID", wantErr: ErrorResourceNotFound, }, + { + creds: credsWithMultipleAudiences, + id: "ID-1", + }, } for i, tt := range tests { @@ -318,6 +347,7 @@ func TestCreateUser(t *testing.T) { cantEmail bool wantResponse schema.UserCreateResponse + wantClientID string wantErr error }{ { @@ -340,6 +370,29 @@ func TestCreateUser(t *testing.T) { CreatedAt: clock.Now().Format(time.RFC3339), }, }, + wantClientID: goodClientID, + }, + { + creds: credsWithMultipleAudiences, + usr: schema.User{ + Email: "newuser01@example.com", + DisplayName: "New User", + EmailVerified: true, + Admin: false, + }, + redirURL: validRedirURL, + + wantResponse: schema.UserCreateResponse{ + EmailSent: true, + User: &schema.User{ + Email: "newuser01@example.com", + DisplayName: "New User", + EmailVerified: true, + Admin: false, + CreatedAt: clock.Now().Format(time.RFC3339), + }, + }, + wantClientID: goodClientID, }, { creds: goodCreds, @@ -362,6 +415,7 @@ func TestCreateUser(t *testing.T) { }, ResetPasswordLink: resetPasswordURL.String(), }, + wantClientID: goodClientID, }, { creds: goodCreds, @@ -397,6 +451,7 @@ func TestCreateUser(t *testing.T) { if tt.wantErr != nil { if err != tt.wantErr { t.Errorf("case %d: want=%q, got=%q", i, tt.wantErr, err) + continue } tok := "" @@ -420,11 +475,13 @@ func TestCreateUser(t *testing.T) { } if err != nil { t.Errorf("case %d: want nil err, got: %q ", i, err) + continue } newID := response.User.Id if newID == "" { t.Errorf("case %d: expected non-empty newID", i) + continue } tt.wantResponse.User.Id = newID @@ -436,7 +493,7 @@ func TestCreateUser(t *testing.T) { wantEmalier := testEmailer{ cantEmail: tt.cantEmail, lastEmail: tt.usr.Email, - lastClientID: tt.creds.ClientID, + lastClientID: tt.wantClientID, lastRedirectURL: tt.redirURL, lastWasInvite: true, } @@ -497,6 +554,7 @@ func TestResendEmailInvitation(t *testing.T) { wantResponse schema.ResendEmailInvitationResponse wantErr error + wantClientID string }{ { creds: goodCreds, @@ -507,6 +565,7 @@ func TestResendEmailInvitation(t *testing.T) { wantResponse: schema.ResendEmailInvitationResponse{ EmailSent: true, }, + wantClientID: goodClientID, }, { creds: goodCreds, @@ -519,6 +578,20 @@ func TestResendEmailInvitation(t *testing.T) { EmailSent: false, ResetPasswordLink: resetPasswordURL.String(), }, + wantClientID: goodClientID, + }, + { + creds: credsWithMultipleAudiences, + userID: "ID-1", + email: "id1@example.com", + redirURL: validRedirURL, + cantEmail: true, + + wantResponse: schema.ResendEmailInvitationResponse{ + EmailSent: false, + ResetPasswordLink: resetPasswordURL.String(), + }, + wantClientID: goodClientID, }, { creds: badCreds, @@ -576,7 +649,7 @@ func TestResendEmailInvitation(t *testing.T) { wantEmailer := testEmailer{ cantEmail: tt.cantEmail, lastEmail: tt.email, - lastClientID: tt.creds.ClientID, + lastClientID: tt.wantClientID, lastRedirectURL: tt.redirURL, lastWasInvite: true, }