TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-08-11 21:07:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(oidc): IDP and machine user auth methods (#7992)

Browse Source 
# Which Problems Are Solved

After https://github.com/zitadel/zitadel/pull/7822 was merged we
discovered that
v2 tokens that where obtained through an IDP using the v1 login, can't
be used for
zitadel API calls.

- Because we used to store the AMR claim on the auth request, but
internally use the domain.UserAuthMethod type. AMR has no notion of an
IDP login, so that "factor" was lost
during conversion. Rendering those v2 tokens invalid on the zitadel API.
- A wrong check on machine user tokens falsly allowed some tokens to be
valid
- The client ID was set to tokens from client credentials and JWT
profile, which made client queries fail in the validation middleware.
The middleware expects client ID unset for machine users.

# How the Problems Are Solved

Store the domain.AuthMethods directly in  the auth requests and session,
instead of using AMR claims with lossy conversion.

- IDPs have seperate auth method, which is not an AMR claim
- Machine users are treated specialy, eg auth methods are not required.
- Do not set the client ID for client credentials and JWT profile

# Additional Changes

Cleaned up mostly unused `oidc.getInfoFromRequest()`.

# Additional Context

- Bugs were introduced in https://github.com/zitadel/zitadel/pull/7822
and not yet part of a release.
- Reported internally.
This commit is contained in:
Tim Möhlmann
2024-05-23 07:35:10 +02:00
committed by GitHub
parent e57a9b57c8
commit f5e9d4f57f
11 changed files with 126 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
internal/api/oidc/auth_request.go
View File

@@ -12,7 +12,6 @@ import (
"github.com/zitadel/logging"
"github.com/zitadel/oidc/v3/pkg/oidc"
"github.com/zitadel/oidc/v3/pkg/op"
"golang.org/x/text/language"
"github.com/zitadel/zitadel/internal/api/authz"
http_utils "github.com/zitadel/zitadel/internal/api/http"
@@ -199,23 +198,6 @@ func (*OPStorage) panicErr(method string) error {
return fmt.Errorf("OPStorage.%s should not be called anymore. This is a bug. Please report https://github.com/zitadel/zitadel/issues", method)
}
func getInfoFromRequest(req op.TokenRequest) (agentID, clientID, userOrgID string, authTime time.Time, amr []string, preferredLanguage *language.Tag, reason domain.TokenReason, actor *domain.TokenActor) {
switch r := req.(type) {
case *AuthRequest:
return r.AgentID, r.ApplicationID, r.UserOrgID, r.AuthTime, r.GetAMR(), r.PreferredLanguage, domain.TokenReasonAuthRequest, nil
case *RefreshTokenRequest:
return r.UserAgentID, r.ClientID, "", r.AuthTime, r.AuthMethodsReferences, nil, domain.TokenReasonRefresh, r.Actor
case op.IDTokenRequest:
return "", r.GetClientID(), "", r.GetAuthTime(), r.GetAMR(), nil, domain.TokenReasonAuthRequest, nil
case *oidc.JWTTokenRequest:
return "", "", "", r.GetAuthTime(), nil, nil, domain.TokenReasonJWTProfile, nil
case *clientCredentialsRequest:
return "", "", "", time.Time{}, nil, nil, domain.TokenReasonClientCredentials, nil
default:
return "", "", "", time.Time{}, nil, nil, domain.TokenReasonAuthRequest, nil
}
}
func (o *OPStorage) TokenRequestByRefreshToken(ctx context.Context, refreshToken string) (_ op.RefreshTokenRequest, err error) {
panic("TokenRequestByRefreshToken should not be called anymore. This is a bug. Please report https://github.com/zitadel/zitadel/issues")
}
@@ -511,8 +493,8 @@ func implicitFlowComplianceChecker() command.AuthRequestComplianceChecker {
func (s *Server) authorizeCallbackHandler(w http.ResponseWriter, r *http.Request) {
authorizer := s.Provider()
authReq, err := func() (authReq op.AuthRequest, err error) {
ctx, span := tracing.NewSpan(r.Context())
authReq, err := func(ctx context.Context) (authReq *AuthRequest, err error) {
ctx, span := tracing.NewSpan(ctx)
r = r.WithContext(ctx)
defer func() { span.EndWithError(err) }()
@@ -520,7 +502,7 @@ func (s *Server) authorizeCallbackHandler(w http.ResponseWriter, r *http.Request
if err != nil {
return nil, err
}
authReq, err = authorizer.Storage().AuthRequestByID(r.Context(), id)
authReq, err = s.getAuthRequestV1ByID(ctx, id)
if err != nil {
return nil, err
}
@@ -528,13 +510,13 @@ func (s *Server) authorizeCallbackHandler(w http.ResponseWriter, r *http.Request
return authReq, oidc.ErrInteractionRequired().WithDescription("Unfortunately, the user may be not logged in and/or additional interaction is required.")
}
return authReq, s.authResponse(authReq, authorizer, w, r)
}()
}(r.Context())
if err != nil {
op.AuthRequestError(w, r, authReq, err, authorizer)
}
}
func (s *Server) authResponse(authReq op.AuthRequest, authorizer op.Authorizer, w http.ResponseWriter, r *http.Request) (err error) {
func (s *Server) authResponse(authReq *AuthRequest, authorizer op.Authorizer, w http.ResponseWriter, r *http.Request) (err error) {
ctx, span := tracing.NewSpan(r.Context())
r = r.WithContext(ctx)
defer func() { span.EndWithError(err) }()
@@ -551,7 +533,7 @@ func (s *Server) authResponse(authReq op.AuthRequest, authorizer op.Authorizer,
return s.authResponseToken(authReq, authorizer, client, w, r)
}
func (s *Server) authResponseToken(authReq op.AuthRequest, authorizer op.Authorizer, opClient op.Client, w http.ResponseWriter, r *http.Request) (err error) {
func (s *Server) authResponseToken(authReq *AuthRequest, authorizer op.Authorizer, opClient op.Client, w http.ResponseWriter, r *http.Request) (err error) {
ctx, span := tracing.NewSpan(r.Context())
r = r.WithContext(ctx)
defer func() { span.EndWithError(err) }()
@@ -561,23 +543,20 @@ func (s *Server) authResponseToken(authReq op.AuthRequest, authorizer op.Authori
return zerrors.ThrowInternal(nil, "OIDC-waeN6", "Error.Internal")
}
userAgentID, _, userOrgID, authTime, authMethodsReferences, preferredLanguage, reason, actor := getInfoFromRequest(authReq)
scope := authReq.GetScopes()
session, err := s.command.CreateOIDCSession(ctx,
authReq.GetSubject(),
userOrgID,
authReq.UserID,
authReq.UserOrgID,
client.client.ClientID,
scope,
authReq.GetAudience(),
AMRToAuthMethodTypes(authMethodsReferences),
authTime,
authReq.Audience,
authReq.AuthMethods(),
authReq.AuthTime,
authReq.GetNonce(),
preferredLanguage,
&domain.UserAgent{
FingerprintID: &userAgentID,
},
reason,
actor,
authReq.PreferredLanguage,
authReq.BrowserInfo.ToUserAgent(),
domain.TokenReasonAuthRequest,
nil,
slices.Contains(scope, oidc.ScopeOfflineAccess),
)
if err != nil {