From 9d5cd12cd4daffafb1c502db0bea603a2403e12d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Thu, 21 Mar 2024 19:42:44 +0200 Subject: [PATCH] fix(oidc): define audience inside auth request instead of token creation (#7610) fix(oidc): define audience inside auth request instead off token creation When using the v1 OIDC Code flow, tokens would not carry the correct audience when returned as JWT. This applies to access tokens as JWT and ID tokens. Introspection would still show the correct audience. This happened because project audience was appended at token creation time. This stored the appended audience, used later in introspection or token refresh. However, the OIDC library still only had a view of the original auth request with the original audience. When signing JWTs it would use this outdated information. This change moves audience modifications to the auth request creation. This is was already the way it was done for v2 login and now v1 follows the same method. Co-authored-by: Livio Spring --- internal/api/oidc/auth_request.go | 31 ++++++++++++------- internal/api/oidc/auth_request_converter.go | 3 +- .../eventsourcing/eventstore/auth_request.go | 9 ------ internal/command/user.go | 2 -- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/internal/api/oidc/auth_request.go b/internal/api/oidc/auth_request.go index 08602538f7..45796782f6 100644 --- a/internal/api/oidc/auth_request.go +++ b/internal/api/oidc/auth_request.go @@ -41,20 +41,28 @@ func (o *OPStorage) CreateAuthRequest(ctx context.Context, req *oidc.AuthRequest return o.createAuthRequest(ctx, req, userID) } -func (o *OPStorage) createAuthRequestLoginClient(ctx context.Context, req *oidc.AuthRequest, hintUserID, loginClient string) (op.AuthRequest, error) { +func (o *OPStorage) createAuthRequestScopeAndAudience(ctx context.Context, req *oidc.AuthRequest) (scope, audience []string, err error) { project, err := o.query.ProjectByClientID(ctx, req.ClientID) if err != nil { - return nil, err + return nil, nil, err } - scope, err := o.assertProjectRoleScopesByProject(ctx, project, req.Scopes) + scope, err = o.assertProjectRoleScopesByProject(ctx, project, req.Scopes) if err != nil { - return nil, err - } - audience, err := o.audienceFromProjectID(ctx, project.ID) - if err != nil { - return nil, err + return nil, nil, err } + audience, err = o.audienceFromProjectID(ctx, project.ID) audience = domain.AddAudScopeToAudience(ctx, audience, scope) + if err != nil { + return nil, nil, err + } + return scope, audience, nil +} + +func (o *OPStorage) createAuthRequestLoginClient(ctx context.Context, req *oidc.AuthRequest, hintUserID, loginClient string) (op.AuthRequest, error) { + scope, audience, err := o.createAuthRequestScopeAndAudience(ctx, req) + if err != nil { + return nil, err + } authRequest := &command.AuthRequest{ LoginClient: loginClient, ClientID: req.ClientID, @@ -88,11 +96,12 @@ func (o *OPStorage) createAuthRequest(ctx context.Context, req *oidc.AuthRequest if !ok { return nil, zerrors.ThrowPreconditionFailed(nil, "OIDC-sd436", "no user agent id") } - req.Scopes, err = o.assertProjectRoleScopes(ctx, req.ClientID, req.Scopes) + scope, audience, err := o.createAuthRequestScopeAndAudience(ctx, req) if err != nil { - return nil, zerrors.ThrowPreconditionFailed(err, "OIDC-Gqrfg", "Errors.Internal") + return nil, err } - authRequest := CreateAuthRequestToBusiness(ctx, req, userAgentID, userID) + req.Scopes = scope + authRequest := CreateAuthRequestToBusiness(ctx, req, userAgentID, userID, audience) resp, err := o.repo.CreateAuthRequest(ctx, authRequest) if err != nil { return nil, err diff --git a/internal/api/oidc/auth_request_converter.go b/internal/api/oidc/auth_request_converter.go index bbe3b1892a..76cbc655dc 100644 --- a/internal/api/oidc/auth_request_converter.go +++ b/internal/api/oidc/auth_request_converter.go @@ -101,7 +101,7 @@ func AuthRequestFromBusiness(authReq *domain.AuthRequest) (_ op.AuthRequest, err return &AuthRequest{authReq}, nil } -func CreateAuthRequestToBusiness(ctx context.Context, authReq *oidc.AuthRequest, userAgentID, userID string) *domain.AuthRequest { +func CreateAuthRequestToBusiness(ctx context.Context, authReq *oidc.AuthRequest, userAgentID, userID string, audience []string) *domain.AuthRequest { return &domain.AuthRequest{ CreationDate: time.Now(), AgentID: userAgentID, @@ -117,6 +117,7 @@ func CreateAuthRequestToBusiness(ctx context.Context, authReq *oidc.AuthRequest, MaxAuthAge: MaxAgeToBusiness(authReq.MaxAge), UserID: userID, InstanceID: authz.GetInstance(ctx).InstanceID(), + Audience: audience, Request: &domain.AuthRequestOIDC{ Scopes: authReq.Scopes, ResponseType: ResponseTypeToBusiness(authReq.ResponseType), diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index 1f65bc42fd..4dc54fcdc8 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -133,15 +133,6 @@ func (repo *AuthRequestRepo) CreateAuthRequest(ctx context.Context, request *dom if err != nil { return nil, err } - projectIDQuery, err := query.NewAppProjectIDSearchQuery(project.ID) - if err != nil { - return nil, err - } - appIDs, err := repo.Query.SearchClientIDs(ctx, &query.AppSearchQueries{Queries: []query.SearchQuery{projectIDQuery}}, false) - if err != nil { - return nil, err - } - request.Audience = appIDs request.AppendAudIfNotExisting(project.ID) request.ApplicationResourceOwner = project.ResourceOwner request.PrivateLabelingSetting = project.PrivateLabelingSetting diff --git a/internal/command/user.go b/internal/command/user.go index 9376f011f7..6d22b212e0 100644 --- a/internal/command/user.go +++ b/internal/command/user.go @@ -297,8 +297,6 @@ func (c *Commands) addUserToken(ctx context.Context, userWriteModel *UserWriteMo cmds = append(cmds, user.NewUserImpersonatedEvent(ctx, userAgg, clientID, actor)) } - audience = domain.AddAudScopeToAudience(ctx, audience, scopes) - preferredLanguage := "" existingHuman, err := c.getHumanWriteModelByID(ctx, userWriteModel.AggregateID, userWriteModel.ResourceOwner) if err != nil {