Merge commit from fork

* fix: add `Scopes` to `Request` interface so that scopes can be validated on all requests

* feat: assert org from scope exists when authorizing requests

* fix: check all scopes

* comments

* check org on callback creation

* fix tests

* clarifications

* fix scope marshaling

* fix: enfore organization for authrequest in initiation

* fix: filter sessions on the accounts page by organization scope

* add integration tests

---------

Co-authored-by: Livio Spring <[email protected]>
Co-authored-by: Max Peintner <[email protected]>
This commit is contained in:
Wim Van Laer
2026-03-17 12:54:27 +01:00
committed by GitHub
co-authored by Livio Spring Max Peintner
parent aa4432dc99
commit c3a6bdb0bb
21 changed files with 729 additions and 115 deletions
@@ -15,6 +15,7 @@ import (
"github.com/zitadel/oidc/v3/pkg/oidc"
"google.golang.org/protobuf/types/known/timestamppb"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/integration"
"github.com/zitadel/zitadel/pkg/grpc/app"
filter "github.com/zitadel/zitadel/pkg/grpc/filter/v2beta"
@@ -163,6 +164,25 @@ func TestServer_CreateCallback(t *testing.T) {
},
wantErr: true,
},
{
name: "invalid organization",
ctx: CTXLoginClient,
req: &oidc_pb.CreateCallbackRequest{
AuthRequestId: func() string {
org := Instance.CreateOrganization(IAMCTX, integration.OrganizationName(), integration.Email())
_, authRequestID, err := Instance.CreateOIDCAuthRequest(CTXLoginClient, client.GetClientId(), Instance.Users.Get(integration.UserTypeLogin).ID, redirectURI, oidc.ScopeOpenID, domain.OrgIDScope+org.OrganizationId)
require.NoError(t, err)
return authRequestID
}(),
CallbackKind: &oidc_pb.CreateCallbackRequest_Session{
Session: &oidc_pb.Session{
SessionId: sessionResp.GetSessionId(),
SessionToken: sessionResp.GetSessionToken(),
},
},
},
wantErr: true,
},
{
name: "fail callback",
ctx: CTXLoginClient,
@@ -356,6 +376,31 @@ func TestServer_CreateCallback(t *testing.T) {
},
wantErr: false,
},
{
name: "callback with required organization",
ctx: CTXLoginClient,
req: &oidc_pb.CreateCallbackRequest{
AuthRequestId: func() string {
_, authRequestID, err := Instance.CreateOIDCAuthRequest(CTXLoginClient, client.GetClientId(), Instance.Users.Get(integration.UserTypeLogin).ID, redirectURI, oidc.ScopeOpenID, domain.OrgIDScope+Instance.DefaultOrg.Id)
require.NoError(t, err)
return authRequestID
}(),
CallbackKind: &oidc_pb.CreateCallbackRequest_Session{
Session: &oidc_pb.Session{
SessionId: sessionResp.GetSessionId(),
SessionToken: sessionResp.GetSessionToken(),
},
},
},
want: &oidc_pb.CreateCallbackResponse{
CallbackUrl: `oidcintegrationtest:\/\/callback\?code=(.*)&state=state`,
Details: &object.Details{
ChangeDate: timestamppb.Now(),
ResourceOwner: Instance.ID(),
},
},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@@ -749,7 +794,7 @@ func TestServer_AuthorizeOrDenyDeviceAuthorization(t *testing.T) {
ctx: CTXLoginClient,
req: &oidc_pb.AuthorizeOrDenyDeviceAuthorizationRequest{
DeviceAuthorizationId: func() string {
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), "openid")
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), oidc.ScopeOpenID)
require.NoError(t, err)
var id string
retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTXLoginClient, time.Minute)
@@ -776,7 +821,7 @@ func TestServer_AuthorizeOrDenyDeviceAuthorization(t *testing.T) {
ctx: CTXLoginClient,
req: &oidc_pb.AuthorizeOrDenyDeviceAuthorizationRequest{
DeviceAuthorizationId: func() string {
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), "openid")
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), oidc.ScopeOpenID)
require.NoError(t, err)
var id string
retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTXLoginClient, time.Minute)
@@ -798,12 +843,40 @@ func TestServer_AuthorizeOrDenyDeviceAuthorization(t *testing.T) {
},
wantErr: true,
},
{
name: "invalid organization",
ctx: CTXLoginClient,
req: &oidc_pb.AuthorizeOrDenyDeviceAuthorizationRequest{
DeviceAuthorizationId: func() string {
org := Instance.CreateOrganization(IAMCTX, integration.OrganizationName(), integration.Email())
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), oidc.ScopeOpenID, domain.OrgIDScope+org.OrganizationId)
require.NoError(t, err)
var id string
retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTXLoginClient, time.Minute)
require.EventuallyWithT(t, func(collectT *assert.CollectT) {
resp, err := Instance.Client.OIDCv2.GetDeviceAuthorizationRequest(CTXLoginClient, &oidc_pb.GetDeviceAuthorizationRequestRequest{
UserCode: req.UserCode,
})
assert.NoError(collectT, err)
id = resp.GetDeviceAuthorizationRequest().GetId()
}, retryDuration, tick)
return id
}(),
Decision: &oidc_pb.AuthorizeOrDenyDeviceAuthorizationRequest_Session{
Session: &oidc_pb.Session{
SessionId: sessionResp.GetSessionId(),
SessionToken: sessionResp.GetSessionToken(),
},
},
},
wantErr: true,
},
{
name: "deny device authorization",
ctx: CTXLoginClient,
req: &oidc_pb.AuthorizeOrDenyDeviceAuthorizationRequest{
DeviceAuthorizationId: func() string {
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), "openid")
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), oidc.ScopeOpenID)
require.NoError(t, err)
var id string
retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTXLoginClient, time.Minute)
@@ -826,7 +899,7 @@ func TestServer_AuthorizeOrDenyDeviceAuthorization(t *testing.T) {
ctx: CTX,
req: &oidc_pb.AuthorizeOrDenyDeviceAuthorizationRequest{
DeviceAuthorizationId: func() string {
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), "openid")
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), oidc.ScopeOpenID)
require.NoError(t, err)
var id string
retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTXLoginClient, time.Minute)
@@ -849,7 +922,7 @@ func TestServer_AuthorizeOrDenyDeviceAuthorization(t *testing.T) {
ctx: CTX,
req: &oidc_pb.AuthorizeOrDenyDeviceAuthorizationRequest{
DeviceAuthorizationId: func() string {
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), "openid")
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), oidc.ScopeOpenID)
require.NoError(t, err)
var id string
retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTXLoginClient, time.Minute)
@@ -876,7 +949,34 @@ func TestServer_AuthorizeOrDenyDeviceAuthorization(t *testing.T) {
ctx: CTXLoginClient,
req: &oidc_pb.AuthorizeOrDenyDeviceAuthorizationRequest{
DeviceAuthorizationId: func() string {
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), "openid")
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), oidc.ScopeOpenID)
require.NoError(t, err)
var id string
retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTXLoginClient, time.Minute)
require.EventuallyWithT(t, func(collectT *assert.CollectT) {
resp, err := Instance.Client.OIDCv2.GetDeviceAuthorizationRequest(CTXLoginClient, &oidc_pb.GetDeviceAuthorizationRequestRequest{
UserCode: req.UserCode,
})
assert.NoError(collectT, err)
id = resp.GetDeviceAuthorizationRequest().GetId()
}, retryDuration, tick)
return id
}(),
Decision: &oidc_pb.AuthorizeOrDenyDeviceAuthorizationRequest_Session{
Session: &oidc_pb.Session{
SessionId: sessionResp.GetSessionId(),
SessionToken: sessionResp.GetSessionToken(),
},
},
},
wantErr: false,
},
{
name: "authorize, with organization scope",
ctx: CTXLoginClient,
req: &oidc_pb.AuthorizeOrDenyDeviceAuthorizationRequest{
DeviceAuthorizationId: func() string {
req, err := Instance.CreateDeviceAuthorizationRequest(CTXLoginClient, client.GetClientId(), oidc.ScopeOpenID, domain.OrgIDScope+Instance.DefaultOrg.GetId())
require.NoError(t, err)
var id string
retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTXLoginClient, time.Minute)
+48 -7
View File
@@ -80,25 +80,31 @@ func (o *OPStorage) CreateAuthRequest(ctx context.Context, req *oidc.AuthRequest
}
}
func (o *OPStorage) createAuthRequestScopeAndAudience(ctx context.Context, clientID string, reqScope []string) (scope, audience []string, err error) {
func (o *OPStorage) createAuthRequestScopeAndAudience(ctx context.Context, clientID string, reqScope []string) (scope, audience []string, orgID string, err error) {
project, err := o.query.ProjectByClientID(ctx, clientID)
if err != nil {
return nil, nil, err
return nil, nil, "", err
}
orgID, err = o.assertOrgScope(ctx, reqScope)
if err != nil {
return nil, nil, "", err
}
scope, err = o.assertProjectRoleScopesByProject(ctx, project, reqScope)
if err != nil {
return nil, 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 nil, nil, "", err
}
return scope, audience, nil
return scope, audience, orgID, nil
}
func (o *OPStorage) createAuthRequestLoginClient(ctx context.Context, req *oidc.AuthRequest, hintUserID, loginClient string) (op.AuthRequest, error) {
scope, audience, err := o.createAuthRequestScopeAndAudience(ctx, req.ClientID, req.Scopes)
scope, audience, orgID, err := o.createAuthRequestScopeAndAudience(ctx, req.ClientID, req.Scopes)
if err != nil {
return nil, err
}
@@ -118,6 +124,7 @@ func (o *OPStorage) createAuthRequestLoginClient(ctx context.Context, req *oidc.
UILocales: UILocalesToBusiness(req.UILocales),
MaxAge: MaxAgeToBusiness(req.MaxAge),
Issuer: o.contextToIssuer(ctx),
OrganizationID: orgID,
}
if req.LoginHint != "" {
authRequest.LoginHint = &req.LoginHint
@@ -138,7 +145,8 @@ func (o *OPStorage) createAuthRequest(ctx context.Context, req *oidc.AuthRequest
if !ok {
return nil, zerrors.ThrowPreconditionFailed(nil, "OIDC-sd436", "no user agent id")
}
scope, audience, err := o.createAuthRequestScopeAndAudience(ctx, req.ClientID, req.Scopes)
// we do not need to handle the orgID for the v1 login, since it handles it already
scope, audience, _, err := o.createAuthRequestScopeAndAudience(ctx, req.ClientID, req.Scopes)
if err != nil {
return nil, err
}
@@ -530,6 +538,39 @@ func (o *OPStorage) GetRefreshTokenInfo(ctx context.Context, clientID string, to
return refreshToken.UserID, refreshToken.ID, nil
}
// assertOrgScope checks the scopes for organization scopes and returns the orgID if exactly one org scope is found.
// If multiple org scopes are found or if the org scope is invalid, an error is returned.
// For backwards compatibility, we support both orgID and orgDomain scopes, but they need to be consistent,
// meaning that if both are provided, they need to belong to the same organization.
func (o *OPStorage) assertOrgScope(ctx context.Context, scopes []string) (string, error) {
var id string
for _, scope := range scopes {
if orgID, ok := strings.CutPrefix(scope, domain.OrgIDScope); ok {
org, err := o.query.OrgByID(ctx, orgID)
if err != nil {
return "", err
}
if id != "" && id != org.ID {
return "", oidc.ErrInvalidScope().WithDescription("Only one organization scope may be provided")
}
id = org.ID
continue
}
if orgDomain, ok := strings.CutPrefix(scope, domain.OrgDomainPrimaryScope); ok {
org, err := o.query.OrgByPrimaryDomain(ctx, orgDomain)
if err != nil {
return "", err
}
if id != "" && id != org.ID {
return "", oidc.ErrInvalidScope().WithDescription("Only one organization scope may be provided")
}
id = org.ID
}
}
return id, nil
}
func (o *OPStorage) assertProjectRoleScopesByProject(ctx context.Context, project *query.Project, scopes []string) ([]string, error) {
for _, scope := range scopes {
if strings.HasPrefix(scope, ScopeProjectRolePrefix) {
+7 -7
View File
@@ -119,13 +119,13 @@ func CreateAuthRequestToBusiness(ctx context.Context, authReq *oidc.AuthRequest,
UserID: userID,
InstanceID: authz.GetInstance(ctx).InstanceID(),
Audience: audience,
Request: &domain.AuthRequestOIDC{
Scopes: authReq.Scopes,
ResponseType: ResponseTypeToBusiness(authReq.ResponseType),
ResponseMode: ResponseModeToBusiness(authReq.ResponseMode),
Nonce: authReq.Nonce,
CodeChallenge: CodeChallengeToBusiness(authReq.CodeChallenge, authReq.CodeChallengeMethod),
},
Request: domain.NewAuthRequestOIDC(
authReq.Scopes,
ResponseTypeToBusiness(authReq.ResponseType),
ResponseModeToBusiness(authReq.ResponseMode),
authReq.Nonce,
CodeChallengeToBusiness(authReq.CodeChallenge, authReq.CodeChallengeMethod),
),
}
}
+2 -2
View File
@@ -76,11 +76,11 @@ func (o *OPStorage) StoreDeviceAuthorization(ctx context.Context, clientID, devi
logger.OnError(err).Error(logMsg)
span.EndWithError(err)
}()
scope, audience, err := o.createAuthRequestScopeAndAudience(ctx, clientID, scope)
scope, audience, orgID, err := o.createAuthRequestScopeAndAudience(ctx, clientID, scope)
if err != nil {
return err
}
details, err := o.command.AddDeviceAuth(ctx, clientID, deviceCode, userCode, expires, scope, audience, slices.Contains(scope, oidc.ScopeOfflineAccess))
details, err := o.command.AddDeviceAuth(ctx, clientID, deviceCode, userCode, orgID, expires, scope, audience, slices.Contains(scope, oidc.ScopeOfflineAccess))
if err == nil {
logger.SetFields("details", details).Debug(logMsg)
}
@@ -17,6 +17,7 @@ import (
http_utils "github.com/zitadel/zitadel/internal/api/http"
oidc_api "github.com/zitadel/zitadel/internal/api/oidc"
"github.com/zitadel/zitadel/internal/command"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/integration"
oidc_pb "github.com/zitadel/zitadel/pkg/grpc/oidc/v2"
"github.com/zitadel/zitadel/pkg/grpc/session/v2"
@@ -36,6 +37,20 @@ func TestOPStorage_CreateAuthRequest(t *testing.T) {
id2 := createAuthRequestNoLoginClientHeader(t, Instance, clientIDV2, redirectURI)
require.Contains(t, id2, command.IDPrefixV2)
// valid org scope must succeed
_, _, err := Instance.CreateOIDCAuthRequest(CTX, clientID, Instance.Users.Get(integration.UserTypeLogin).ID, redirectURI, oidc.ScopeOpenID, domain.OrgIDScope+Instance.DefaultOrg.Id)
require.NoError(t, err)
_, _, err = Instance.CreateOIDCAuthRequest(CTX, clientID, Instance.Users.Get(integration.UserTypeLogin).ID, redirectURI, oidc.ScopeOpenID, domain.OrgIDScope+Instance.DefaultOrg.Id)
require.NoError(t, err)
// invalid org scope must fail
_, _, err = Instance.CreateOIDCAuthRequest(CTX, clientID, Instance.Users.Get(integration.UserTypeLogin).ID, redirectURI, oidc.ScopeOpenID, domain.OrgIDScope+"invalid")
require.Error(t, err)
_, _, err = Instance.CreateOIDCAuthRequest(CTX, clientID, Instance.Users.Get(integration.UserTypeLogin).ID, redirectURI, oidc.ScopeOpenID, domain.OrgIDScope+"invalid")
require.Error(t, err)
}
func TestOPStorage_CreateAccessToken_code(t *testing.T) {