Skip to content

Commit

Permalink
Do not fail when uid/gid are missing
Browse files Browse the repository at this point in the history
  • Loading branch information
glpatcern committed Apr 20, 2022
1 parent 0642bd6 commit 957927c
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 13 deletions.
7 changes: 4 additions & 3 deletions changelog/unreleased/oidc-fix.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
Bugfix: made uid, gid claims parsing more robust in OIDC auth provider

This fix makes sure the uid and gid claims are defined at init time and that
a proper error is returned in case they would be missing or invalid (i.e. not int64)
when authenticating users.
This fix makes sure the uid and gid claims are defined at init time, and that
the necessary typecasts are performed correctly when authenticating users.
A comment was added that in case the uid/gid claims are missing AND that no
mapping takes place, a user entity is returned with uid = gid = 0.

https://github.com/cs3org/reva/pull/2759
16 changes: 6 additions & 10 deletions pkg/auth/manager/oidc/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,16 +198,12 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
if claims["email"] == nil {
return nil, nil, fmt.Errorf("no \"email\" attribute found in userinfo: maybe the client did not request the oidc \"email\"-scope")
}
if uid, ok := claims[am.c.UIDClaim].(float64); ok {
claims[am.c.UIDClaim] = int64(uid)
} else {
return nil, nil, fmt.Errorf("malformed or missing uid claim in userinfo: '%v'", claims[am.c.UIDClaim])
}
if gid, ok := claims[am.c.GIDClaim].(float64); ok {
claims[am.c.GIDClaim] = int64(gid)
} else {
return nil, nil, fmt.Errorf("malformed or missing gid claim in userinfo: '%v'", claims[am.c.GIDClaim])
}

uid, _ := claims[am.c.UIDClaim].(float64)
claims[am.c.UIDClaim] = int64(uid) // in case the uid claim is missing and a mapping is to be performed, resolveUser() will populate it
// Note that if not, will silently carry a user with 0 uid, potentially problematic with storage providers
gid, _ := claims[am.c.GIDClaim].(float64)
claims[am.c.GIDClaim] = int64(gid)

err = am.resolveUser(ctx, claims)
if err != nil {
Expand Down

0 comments on commit 957927c

Please sign in to comment.