account_resolver: Handle user roles separately from user lookup
This removes the "withRoles" flag from the GetUserByClaims lookup and move the functionality into a separate method. This should make the code a bit more readable in preparation for maintaining the RoleAssignments from OIDC claims.
This commit is contained in:
committed by
Ralf Haferkamp
parent
f5cfa7e126
commit
88e8bb1b72
@@ -62,7 +62,7 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
user, token, err = m.userProvider.GetUserByClaims(req.Context(), m.userCS3Claim, value, true)
|
||||
user, token, err = m.userProvider.GetUserByClaims(req.Context(), m.userCS3Claim, value)
|
||||
|
||||
if errors.Is(err, backend.ErrAccountNotFound) {
|
||||
m.logger.Debug().Str("claim", m.userOIDCClaim).Str("value", value).Msg("User by claim not found")
|
||||
@@ -76,7 +76,7 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
|
||||
if err != nil {
|
||||
m.logger.Error().Err(err).Msg("Autoprovisioning user failed")
|
||||
}
|
||||
user, token, err = m.userProvider.GetUserByClaims(req.Context(), "userid", user.Id.OpaqueId, true)
|
||||
user, token, err = m.userProvider.GetUserByClaims(req.Context(), "userid", user.Id.OpaqueId)
|
||||
if err != nil {
|
||||
m.logger.Error().Err(err).Str("userid", user.Id.OpaqueId).Msg("Error getting token for autoprovisioned user")
|
||||
}
|
||||
@@ -94,6 +94,14 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// resolve the user's roles
|
||||
user, err = m.userProvider.GetUserRoles(ctx, user)
|
||||
if err != nil {
|
||||
m.logger.Error().Err(err).Msg("Could not get user roles")
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
// add user to context for selectors
|
||||
ctx = revactx.ContextSetUser(ctx, user)
|
||||
req = req.WithContext(ctx)
|
||||
@@ -101,7 +109,7 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
|
||||
m.logger.Debug().Interface("claims", claims).Interface("user", user).Msg("associated claims with user")
|
||||
} else if user != nil {
|
||||
var err error
|
||||
_, token, err = m.userProvider.GetUserByClaims(req.Context(), "username", user.Username, true)
|
||||
_, token, err = m.userProvider.GetUserByClaims(req.Context(), "username", user.Username)
|
||||
|
||||
if errors.Is(err, backend.ErrAccountDisabled) {
|
||||
m.logger.Debug().Interface("user", user).Msg("Disabled")
|
||||
|
||||
@@ -122,6 +122,7 @@ func newMockAccountResolver(userBackendResult *userv1beta1.User, userBackendErr
|
||||
|
||||
ub := mocks.UserBackend{}
|
||||
ub.On("GetUserByClaims", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(userBackendResult, token, userBackendErr)
|
||||
ub.On("GetUserRoles", mock.Anything, mock.Anything).Return(userBackendResult, nil)
|
||||
|
||||
return AccountResolver(
|
||||
Logger(log.NewLogger()),
|
||||
|
||||
@@ -192,7 +192,17 @@ func (m SignedURLAuthenticator) Authenticate(r *http.Request) (*http.Request, bo
|
||||
return nil, false
|
||||
}
|
||||
|
||||
user, _, err := m.UserProvider.GetUserByClaims(r.Context(), "username", r.URL.Query().Get(_paramOCCredential), true)
|
||||
user, _, err := m.UserProvider.GetUserByClaims(r.Context(), "username", r.URL.Query().Get(_paramOCCredential))
|
||||
if err != nil {
|
||||
m.Logger.Error().
|
||||
Err(err).
|
||||
Str("authenticator", "signed_url").
|
||||
Str("path", r.URL.Path).
|
||||
Msg("Could not get user by claim")
|
||||
return nil, false
|
||||
}
|
||||
|
||||
user, err = m.UserProvider.GetUserRoles(r.Context(), user)
|
||||
if err != nil {
|
||||
m.Logger.Error().
|
||||
Err(err).
|
||||
|
||||
Reference in New Issue
Block a user