diff --git a/internal/api/oidc/userinfo.go b/internal/api/oidc/userinfo.go index 0cc0bc1b5b8..7f059cbadf6 100644 --- a/internal/api/oidc/userinfo.go +++ b/internal/api/oidc/userinfo.go @@ -25,13 +25,23 @@ func (s *Server) userInfo(ctx context.Context, userID, projectID string, scope, return nil, err } - userInfo := userInfoToOIDC(projectID, qu, scope, requestedRoles, s.assetAPIPrefix(ctx)) + userInfo := userInfoToOIDC(projectID, qu, scope, roleAudience, requestedRoles, s.assetAPIPrefix(ctx)) return userInfo, s.userinfoFlows(ctx, qu, userInfo) } +// prepareRoles scans the requested scopes, appends to roleAudiendce and returns the requestedRoles. +// +// When [ScopeProjectsRoles] is present and roleAudience was empty, +// project IDs with the [domain.ProjectIDScope] prefix are added to the roleAudience. +// +// Scopes with [ScopeProjectRolePrefix] are added to requestedRoles. +// +// If the resulting requestedRoles or roleAudience are not not empty, +// the current projectID will always be parts or roleAudience. +// Else nil, nil is returned. func prepareRoles(ctx context.Context, projectID string, scope, roleAudience []string) (ra, requestedRoles []string) { // if all roles are requested take the audience for those from the scopes - if slices.Contains(scope, ScopeProjectsRoles) { + if slices.Contains(scope, ScopeProjectsRoles) && len(roleAudience) == 0 { roleAudience = domain.AddAudScopeToAudience(ctx, roleAudience, scope) } requestedRoles = make([]string, 0, len(scope)) @@ -44,14 +54,13 @@ func prepareRoles(ctx context.Context, projectID string, scope, roleAudience []s return nil, nil } - // ensure the projectID of the requesting is part of the roleAudience - if !slices.Contains(roleAudience, projectID) { + if projectID != "" && !slices.Contains(roleAudience, projectID) { roleAudience = append(roleAudience, projectID) } return roleAudience, requestedRoles } -func userInfoToOIDC(projectID string, user *query.OIDCUserInfo, scope, requestedRoles []string, assetPrefix string) *oidc.UserInfo { +func userInfoToOIDC(projectID string, user *query.OIDCUserInfo, scope, roleAudience, requestedRoles []string, assetPrefix string) *oidc.UserInfo { out := new(oidc.UserInfo) for _, s := range scope { switch s { @@ -70,16 +79,20 @@ func userInfoToOIDC(projectID string, user *query.OIDCUserInfo, scope, requested case ScopeResourceOwner: setUserInfoOrgClaims(user, out) default: - if strings.HasPrefix(s, domain.OrgDomainPrimaryScope) { - out.AppendClaims(domain.OrgDomainPrimaryClaim, strings.TrimPrefix(s, domain.OrgDomainPrimaryScope)) + if claim, ok := strings.CutPrefix(s, domain.OrgDomainPrimaryScope); ok { + out.AppendClaims(domain.OrgDomainPrimaryClaim, claim) } - if strings.HasPrefix(s, domain.OrgIDScope) { - out.AppendClaims(domain.OrgIDClaim, strings.TrimPrefix(s, domain.OrgIDScope)) + if claim, ok := strings.CutPrefix(s, domain.OrgIDScope); ok { + out.AppendClaims(domain.OrgIDClaim, claim) setUserInfoOrgClaims(user, out) } } } - setUserInfoRoleClaims(out, newProjectRoles(projectID, user.UserGrants, requestedRoles)) + + // prevent returning obtained grants if none where requested + if (projectID != "" && len(requestedRoles) > 0) || len(roleAudience) > 0 { + setUserInfoRoleClaims(out, newProjectRoles(projectID, user.UserGrants, requestedRoles)) + } return out } @@ -114,10 +127,7 @@ func userInfoProfileToOidc(user *query.User, assetPrefix string) oidc.UserInfoPr PreferredUsername: user.PreferredLoginName, } } - return oidc.UserInfoProfile{ - UpdatedAt: oidc.FromTime(user.ChangeDate), - PreferredUsername: user.PreferredLoginName, - } + return oidc.UserInfoProfile{} } func userInfoPhoneToOIDC(user *query.User) oidc.UserInfoPhone { diff --git a/internal/api/oidc/userinfo_test.go b/internal/api/oidc/userinfo_test.go new file mode 100644 index 00000000000..d9f63218812 --- /dev/null +++ b/internal/api/oidc/userinfo_test.go @@ -0,0 +1,433 @@ +package oidc + +import ( + "context" + "encoding/base64" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/zitadel/oidc/v3/pkg/oidc" + "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/query" + "golang.org/x/text/language" +) + +func Test_prepareRoles(t *testing.T) { + type args struct { + projectID string + scope []string + roleAudience []string + } + tests := []struct { + name string + args args + wantRa []string + wantRequestedRoles []string + }{ + { + name: "empty scope and roleAudience", + args: args{ + projectID: "projID", + scope: nil, + roleAudience: nil, + }, + wantRa: nil, + wantRequestedRoles: nil, + }, + { + name: "some scope and roleAudience", + args: args{ + projectID: "projID", + scope: []string{"openid", "profile"}, + roleAudience: []string{"project2"}, + }, + wantRa: []string{"project2", "projID"}, + wantRequestedRoles: []string{}, + }, + { + name: "scope projects roles", + args: args{ + projectID: "projID", + scope: []string{ScopeProjectsRoles, domain.ProjectIDScope + "project2" + domain.AudSuffix}, + roleAudience: nil, + }, + wantRa: []string{"project2", "projID"}, + wantRequestedRoles: []string{}, + }, + { + name: "scope project role prefix", + args: args{ + projectID: "projID", + scope: []string{"openid", "profile", ScopeProjectRolePrefix + "foo", ScopeProjectRolePrefix + "bar"}, + roleAudience: nil, + }, + wantRa: []string{"projID"}, + wantRequestedRoles: []string{"foo", "bar"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotRa, gotRequestedRoles := prepareRoles(context.Background(), tt.args.projectID, tt.args.scope, tt.args.roleAudience) + assert.Equal(t, tt.wantRa, gotRa, "roleAudience") + assert.Equal(t, tt.wantRequestedRoles, gotRequestedRoles, "requestedRoles") + }) + } +} + +func Test_userInfoToOIDC(t *testing.T) { + metadata := []query.UserMetadata{ + { + Key: "key1", + Value: []byte{1, 2, 3}, + }, + { + Key: "key2", + Value: []byte{4, 5, 6}, + }, + } + organization := &query.UserInfoOrg{ + ID: "orgID", + Name: "orgName", + PrimaryDomain: "orgDomain", + } + humanUserInfo := &query.OIDCUserInfo{ + User: &query.User{ + ID: "human1", + CreationDate: time.Unix(123, 456), + ChangeDate: time.Unix(567, 890), + ResourceOwner: "orgID", + Sequence: 22, + State: domain.UserStateActive, + Type: domain.UserTypeHuman, + Username: "username", + LoginNames: []string{"foo", "bar"}, + PreferredLoginName: "foo", + Human: &query.Human{ + FirstName: "user", + LastName: "name", + NickName: "foobar", + DisplayName: "xxx", + AvatarKey: "picture.png", + PreferredLanguage: language.Dutch, + Gender: domain.GenderDiverse, + Email: "foo@bar.com", + IsEmailVerified: true, + Phone: "+31123456789", + IsPhoneVerified: true, + }, + }, + Metadata: metadata, + Org: organization, + UserGrants: []query.UserGrant{ + { + ID: "ug1", + CreationDate: time.Unix(444, 444), + ChangeDate: time.Unix(555, 555), + Sequence: 55, + Roles: []string{"role1", "role2"}, + GrantID: "grantID", + State: domain.UserGrantStateActive, + UserID: "human1", + Username: "username", + ResourceOwner: "orgID", + ProjectID: "project1", + OrgName: "orgName", + OrgPrimaryDomain: "orgDomain", + ProjectName: "projectName", + UserResourceOwner: "org1", + }, + }, + } + machineUserInfo := &query.OIDCUserInfo{ + User: &query.User{ + ID: "machine1", + CreationDate: time.Unix(123, 456), + ChangeDate: time.Unix(567, 890), + ResourceOwner: "orgID", + Sequence: 23, + State: domain.UserStateActive, + Type: domain.UserTypeMachine, + Username: "machine", + PreferredLoginName: "meanMachine", + Machine: &query.Machine{ + Name: "machine", + Description: "I'm a robot", + }, + }, + Org: organization, + UserGrants: []query.UserGrant{ + { + ID: "ug1", + CreationDate: time.Unix(444, 444), + ChangeDate: time.Unix(555, 555), + Sequence: 55, + Roles: []string{"role1", "role2"}, + GrantID: "grantID", + State: domain.UserGrantStateActive, + UserID: "human1", + Username: "username", + ResourceOwner: "orgID", + ProjectID: "project1", + OrgName: "orgName", + OrgPrimaryDomain: "orgDomain", + ProjectName: "projectName", + UserResourceOwner: "org1", + }, + }, + } + + type args struct { + projectID string + user *query.OIDCUserInfo + scope []string + roleAudience []string + requestedRoles []string + } + tests := []struct { + name string + args args + want *oidc.UserInfo + }{ + { + name: "human, empty", + args: args{ + projectID: "project1", + user: humanUserInfo, + }, + want: &oidc.UserInfo{}, + }, + { + name: "machine, empty", + args: args{ + projectID: "project1", + user: machineUserInfo, + }, + want: &oidc.UserInfo{}, + }, + { + name: "human, scope openid", + args: args{ + projectID: "project1", + user: humanUserInfo, + scope: []string{oidc.ScopeOpenID}, + }, + want: &oidc.UserInfo{ + Subject: "human1", + }, + }, + { + name: "machine, scope openid", + args: args{ + projectID: "project1", + user: machineUserInfo, + scope: []string{oidc.ScopeOpenID}, + }, + want: &oidc.UserInfo{ + Subject: "machine1", + }, + }, + { + name: "human, scope email", + args: args{ + projectID: "project1", + user: humanUserInfo, + scope: []string{oidc.ScopeEmail}, + }, + want: &oidc.UserInfo{ + UserInfoEmail: oidc.UserInfoEmail{ + Email: "foo@bar.com", + EmailVerified: true, + }, + }, + }, + { + name: "machine, scope email", + args: args{ + projectID: "project1", + user: machineUserInfo, + scope: []string{oidc.ScopeEmail}, + }, + want: &oidc.UserInfo{ + UserInfoEmail: oidc.UserInfoEmail{}, + }, + }, + { + name: "human, scope profile", + args: args{ + projectID: "project1", + user: humanUserInfo, + scope: []string{oidc.ScopeProfile}, + }, + want: &oidc.UserInfo{ + UserInfoProfile: oidc.UserInfoProfile{ + Name: "xxx", + GivenName: "user", + FamilyName: "name", + Nickname: "foobar", + Picture: "https://foo.com/assets/orgID/picture.png", + Gender: "diverse", + Locale: oidc.NewLocale(language.Dutch), + UpdatedAt: oidc.FromTime(time.Unix(567, 890)), + PreferredUsername: "foo", + }, + }, + }, + { + name: "machine, scope profile", + args: args{ + projectID: "project1", + user: machineUserInfo, + scope: []string{oidc.ScopeProfile}, + }, + want: &oidc.UserInfo{ + UserInfoProfile: oidc.UserInfoProfile{ + Name: "machine", + UpdatedAt: oidc.FromTime(time.Unix(567, 890)), + PreferredUsername: "meanMachine", + }, + }, + }, + { + name: "human, scope phone", + args: args{ + projectID: "project1", + user: humanUserInfo, + scope: []string{oidc.ScopePhone}, + }, + want: &oidc.UserInfo{ + UserInfoPhone: oidc.UserInfoPhone{ + PhoneNumber: "+31123456789", + PhoneNumberVerified: true, + }, + }, + }, + { + name: "machine, scope phone", + args: args{ + projectID: "project1", + user: machineUserInfo, + scope: []string{oidc.ScopePhone}, + }, + want: &oidc.UserInfo{ + UserInfoPhone: oidc.UserInfoPhone{}, + }, + }, + { + name: "human, scope metadata", + args: args{ + projectID: "project1", + user: humanUserInfo, + scope: []string{ScopeUserMetaData}, + }, + want: &oidc.UserInfo{ + Claims: map[string]any{ + ClaimUserMetaData: map[string]string{ + "key1": base64.RawURLEncoding.EncodeToString([]byte{1, 2, 3}), + "key2": base64.RawURLEncoding.EncodeToString([]byte{4, 5, 6}), + }, + }, + }, + }, + { + name: "machine, scope metadata, none found", + args: args{ + projectID: "project1", + user: machineUserInfo, + scope: []string{ScopeUserMetaData}, + }, + want: &oidc.UserInfo{}, + }, + { + name: "machine, scope resource owner", + args: args{ + projectID: "project1", + user: machineUserInfo, + scope: []string{ScopeResourceOwner}, + }, + want: &oidc.UserInfo{ + Claims: map[string]any{ + ClaimResourceOwner + "id": "orgID", + ClaimResourceOwner + "name": "orgName", + ClaimResourceOwner + "primary_domain": "orgDomain", + }, + }, + }, + { + name: "human, scope org primary domain prefix", + args: args{ + projectID: "project1", + user: humanUserInfo, + scope: []string{domain.OrgDomainPrimaryScope + "foo.com"}, + }, + want: &oidc.UserInfo{ + Claims: map[string]any{ + domain.OrgDomainPrimaryClaim: "foo.com", + }, + }, + }, + { + name: "machine, scope org id", + args: args{ + projectID: "project1", + user: machineUserInfo, + scope: []string{domain.OrgIDScope + "orgID"}, + }, + want: &oidc.UserInfo{ + Claims: map[string]any{ + domain.OrgIDClaim: "orgID", + ClaimResourceOwner + "id": "orgID", + ClaimResourceOwner + "name": "orgName", + ClaimResourceOwner + "primary_domain": "orgDomain", + }, + }, + }, + { + name: "human, roleAudience", + args: args{ + projectID: "project1", + user: humanUserInfo, + roleAudience: []string{"project1"}, + }, + want: &oidc.UserInfo{ + Claims: map[string]any{ + ClaimProjectRoles: projectRoles{ + "role1": {"orgID": "orgDomain"}, + "role2": {"orgID": "orgDomain"}, + }, + fmt.Sprintf(ClaimProjectRolesFormat, "project1"): projectRoles{ + "role1": {"orgID": "orgDomain"}, + "role2": {"orgID": "orgDomain"}, + }, + }, + }, + }, + { + name: "human, requested roles", + args: args{ + projectID: "project1", + user: humanUserInfo, + roleAudience: []string{"project1"}, + requestedRoles: []string{"role2"}, + }, + want: &oidc.UserInfo{ + Claims: map[string]any{ + ClaimProjectRoles: projectRoles{ + "role2": {"orgID": "orgDomain"}, + }, + fmt.Sprintf(ClaimProjectRolesFormat, "project1"): projectRoles{ + "role2": {"orgID": "orgDomain"}, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assetPrefix := "https://foo.com/assets" + got := userInfoToOIDC(tt.args.projectID, tt.args.user, tt.args.scope, tt.args.roleAudience, tt.args.requestedRoles, assetPrefix) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/internal/query/userinfo.go b/internal/query/userinfo.go index a3d7a6a7332..a65420c067f 100644 --- a/internal/query/userinfo.go +++ b/internal/query/userinfo.go @@ -58,11 +58,11 @@ func (q *Queries) GetOIDCUserInfo(ctx context.Context, userID string, roleAudien type OIDCUserInfo struct { User *User `json:"user,omitempty"` Metadata []UserMetadata `json:"metadata,omitempty"` - Org *userInfoOrg `json:"org,omitempty"` + Org *UserInfoOrg `json:"org,omitempty"` UserGrants []UserGrant `json:"user_grants,omitempty"` } -type userInfoOrg struct { +type UserInfoOrg struct { ID string `json:"id,omitempty"` Name string `json:"name,omitempty"` PrimaryDomain string `json:"primary_domain,omitempty"` diff --git a/internal/query/userinfo_test.go b/internal/query/userinfo_test.go index e5ab98d9598..fe0dc749caa 100644 --- a/internal/query/userinfo_test.go +++ b/internal/query/userinfo_test.go @@ -93,7 +93,7 @@ func TestQueries_GetOIDCUserInfo(t *testing.T) { }, Machine: nil, }, - Org: &userInfoOrg{ + Org: &UserInfoOrg{ Name: "demo", PrimaryDomain: "demo.localhost", }, @@ -128,7 +128,7 @@ func TestQueries_GetOIDCUserInfo(t *testing.T) { }, Machine: nil, }, - Org: &userInfoOrg{ + Org: &UserInfoOrg{ Name: "demo", PrimaryDomain: "demo.localhost", }, @@ -185,7 +185,7 @@ func TestQueries_GetOIDCUserInfo(t *testing.T) { }, Machine: nil, }, - Org: &userInfoOrg{ + Org: &UserInfoOrg{ Name: "demo", PrimaryDomain: "demo.localhost", }, @@ -270,7 +270,7 @@ func TestQueries_GetOIDCUserInfo(t *testing.T) { Description: "My test service user", }, }, - Org: &userInfoOrg{ + Org: &UserInfoOrg{ Name: "demo", PrimaryDomain: "demo.localhost", },