WIP: Allow roles to be specified for Keycloak #767
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,16 +2,20 @@ package providers | |
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "net/url" | ||
|
|
||
| "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" | ||
| "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" | ||
| "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" | ||
| "gopkg.in/square/go-jose.v2/jwt" | ||
| ) | ||
|
|
||
| type KeycloakProvider struct { | ||
| *ProviderData | ||
| Group string | ||
| Roles []string | ||
| } | ||
|
|
||
| var _ Provider = (*KeycloakProvider)(nil) | ||
|
|
@@ -63,37 +67,126 @@ func (p *KeycloakProvider) SetGroup(group string) { | |
| p.Group = group | ||
| } | ||
|
|
||
| func (p *KeycloakProvider) SetRoles(roles []string) { | ||
| p.Roles = roles | ||
| } | ||
|
|
||
| type keycloakUserInfo struct { | ||
| Email string `json:"email"` | ||
| Groups []string `json:"groups"` | ||
| } | ||
|
|
||
| func (p *KeycloakProvider) getUserInfo(ctx context.Context, s *sessions.SessionState) (*keycloakUserInfo, error) { | ||
| var userInfo keycloakUserInfo | ||
| err := requests.New(p.ValidateURL.String()). | ||
| WithContext(ctx). | ||
| SetHeader("Authorization", "Bearer "+s.AccessToken). | ||
| Do(). | ||
| UnmarshalInto(&userInfo) | ||
| if err != nil { | ||
| logger.Errorf("failed making request %s", err) | ||
| return nil, fmt.Errorf("error getting user info: %v", err) | ||
| } | ||
| return &userInfo, nil | ||
| } | ||
|
|
||
| func getClientRoles(resourceAccess map[string]interface{}) ([]string, error) { | ||
| var clientRoles []string | ||
| for name, list := range resourceAccess { | ||
| scopes, ok := list.(map[string]interface{}) | ||
| if !ok { | ||
| return nil, errors.New("error parsing client roles from claims") | ||
| } | ||
| if roles, found := scopes["roles"]; found { | ||
| for _, r := range roles.([]interface{}) { | ||
|
There was a problem hiding this comment. Do you need to handle when this isn't a There was a problem hiding this comment. From what I've seen it always returns a slice even when if it only contains a single element, is there any other case that I'm missing? There was a problem hiding this comment. Cool - that makes sense that it is uniform since it is all provided by the singular Keycloak. OIDC is all over the place cause identity providers do some crazy stuff with |
||
| clientRoles = append(clientRoles, fmt.Sprintf("%s:%s", name, r)) | ||
|
There was a problem hiding this comment. Will There was a problem hiding this comment. Oh wait, nevermind - I see the There was a problem hiding this comment. Yes, so effectively this would end up in allowed groups as role:clientname:clientrole. These are client roles which are namespaced by client, so while the structure is predictable the client name could be anything, hence the additional prefix. |
||
| } | ||
| } | ||
| } | ||
| return clientRoles, nil | ||
| } | ||
|
|
||
| type realmAccess struct { | ||
| Roles []string `json:"roles"` | ||
| } | ||
|
|
||
| type customClaims struct { | ||
| RealmAccess realmAccess `json:"realm_access"` | ||
| ResourceAccess map[string]interface{} `json:"resource_access"` | ||
| } | ||
|
|
||
| func (p *KeycloakProvider) getRoles(s *sessions.SessionState) ([]string, error) { | ||
| // Decode JWT token without verifying the signature | ||
| token, err := jwt.ParseSigned(s.AccessToken) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to parse token: %v", err) | ||
| } | ||
| standardClaims := jwt.Claims{} | ||
| customClaims := customClaims{} | ||
| // Parse claims | ||
| if err := token.UnsafeClaimsWithoutVerification(&standardClaims, &customClaims); err != nil { | ||
| logger.Printf("failed to parse claims: %s", err) | ||
| } | ||
| clientRoles, err := getClientRoles(customClaims.ResourceAccess) | ||
| if err != nil { | ||
| logger.Printf("failed to extract client roles: %s", err) | ||
| } | ||
| return append(customClaims.RealmAccess.Roles, clientRoles...), nil | ||
|
There was a problem hiding this comment. It feels weird to pollute & extend the underlying object we extracted from the token claims. I understand the performance motivations of reusing the existing slice since it is done at the end of the function anyway, but since these are small & therefore performance is negligible, maybe clarity is more ideal? Took me a second on first glance to realize the desired output was intended to build the final roles list from There was a problem hiding this comment. You're right, I will change this for better clarity. |
||
| } | ||
|
|
||
| func (p *KeycloakProvider) addGroupsToSession(s *sessions.SessionState, groups []string) error { | ||
| for _, group := range groups { | ||
| s.Groups = append(s.Groups, fmt.Sprintf("group:%s", group)) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func (p *KeycloakProvider) addRolesToSession(s *sessions.SessionState) error { | ||
| roles, err := p.getRoles(s) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| for _, role := range roles { | ||
| s.Groups = append(s.Groups, fmt.Sprintf("role:%s", role)) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
khawaga marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| func (p *KeycloakProvider) EnrichSession(ctx context.Context, s *sessions.SessionState) error { | ||
| userInfo, err := p.getUserInfo(ctx, s) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| s.Email = userInfo.Email | ||
|
|
||
| if p.Group != "" { | ||
| err := p.addGroupsToSession(s, userInfo.Groups) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| if len(p.Roles) > 0 { | ||
| err := p.addRolesToSession(s) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // PrefixAllowedGroups returns a list of allowed groups, prefixed by their `kind` value | ||
| func (p *KeycloakProvider) PrefixAllowedGroups() (groups []string) { | ||
|
|
||
| if p.Group != "" { | ||
| groups = append(groups, fmt.Sprintf("group:%s", p.Group)) | ||
| } | ||
|
|
||
| for _, role := range p.Roles { | ||
| groups = append(groups, fmt.Sprintf("role:%s", role)) | ||
| } | ||
|
|
||
| return groups | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What key was used to sign this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's using the default values on jwt.io and the default secret which is "your-256-bit-secret" 😅
Should I change it to something else? Or add it as a comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm centralizing OIDC logic in this PR and making sample IDTokens more accessible to test cases across providers here: https://github.com/oauth2-proxy/oauth2-proxy/pull/936/files#diff-e719a8f54d2fe3115f6fcaabd056be5b53e86a3f04004d3c53f0b6c5b0409d68R34
Might be of use to this PR if you need valid IDTokens for testing.