Skip to content

Commit

Permalink
chore: address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
mistahj67 committed Oct 24, 2024
1 parent 9bec850 commit e190d7d
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 15 deletions.
11 changes: 1 addition & 10 deletions cmd/api/src/api/saml/saml.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,16 +348,7 @@ func (s ProviderResource) serveAssertionConsumerService(response http.ResponseWr
s.writeAPIErrorResponse(request, response, http.StatusUnauthorized, api.ErrorResponseDetailsAuthenticationInvalid)
} else if principalName, err := s.getSAMLUserPrincipalNameFromAssertion(assertion); err != nil {
log.Errorf("[SAML] Failed to lookup user for SAML provider %s: %v", s.serviceProvider.Config.Name, err)

switch {
case errors.Is(err, ErrorSAMLAssertion):
s.writeAPIErrorResponse(request, response, http.StatusBadRequest, "session assertion does not meet the requirements for user lookup")
case errors.Is(err, ErrorUserNotFound), errors.Is(err, ErrorUserNotAuthorizedForProvider):
// This is a tiny bit more descriptive for the end user without leaking any sensitive information
s.writeAPIErrorResponse(request, response, http.StatusForbidden, "user is not allowed")
default:
s.writeAPIErrorResponse(request, response, http.StatusInternalServerError, "session creation failure")
}
s.writeAPIErrorResponse(request, response, http.StatusBadRequest, "session assertion does not meet the requirements for user lookup")
} else {
s.authenticator.CreateSSOSession(request, response, principalName, s.serviceProvider.Config)
}
Expand Down
10 changes: 5 additions & 5 deletions cmd/api/src/api/v2/auth/sso.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,15 @@ func (s ManagementResource) SSOCallbackHandler(response http.ResponseWriter, req
switch ssoProvider.Type {
case model.SessionAuthProviderSAML:
//todo handle saml callback
api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusNotFound, api.ErrorResponseDetailsResourceNotFound, request), response)
api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusNotImplemented, api.ErrorResponseDetailsNotImplemented, request), response)
case model.SessionAuthProviderOIDC:
if oidcProvider, err := s.db.GetOIDCProviderBySSOProviderID(request.Context(), ssoProvider.ID); err != nil {
api.HandleDatabaseError(request, response, err)
if ssoProvider.OIDCProvider == nil {
api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusNotFound, api.ErrorResponseDetailsResourceNotFound, request), response)
} else {
s.OIDCCallbackHandler(response, request, ssoProvider, oidcProvider)
s.OIDCCallbackHandler(response, request, ssoProvider, *ssoProvider.OIDCProvider)
}
default:
api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusNotFound, api.ErrorResponseDetailsResourceNotFound, request), response)
api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusNotImplemented, api.ErrorResponseDetailsNotImplemented, request), response)
}
}
}

0 comments on commit e190d7d

Please sign in to comment.