Skip to content

Commit 32fee2b

Browse files
committed
refactor(azure/groups): only assign all-users groups if explicitly enabled
1 parent 6498c84 commit 32fee2b

File tree

2 files changed

+11
-14
lines changed

2 files changed

+11
-14
lines changed

docs/lifecycle.md

+5-5
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,12 @@ the `AzureAdApplication` when using the application in a Web API flow such as th
185185

186186
Additionally, any users that should be able to log in to the application using the OpenID Connect flows must be
187187
explicitly assigned to the application through one of the groups defined in `spec.claims.groups[]`. The user _must_ be a
188-
direct member of the group; assignment does not cascade to nested groups.
188+
direct member of the group; assignment does not cascade to nested groups. This also applies to tokens acquired by other
189+
consuming applications through the OAuth 2.0 On-behalf-of flow.
189190

190-
If `spec.claims.groups[]` is not defined, we fall back to assigning a single group (configured by the
191-
`azure.features.groups-assignment.all-users-group-id` flag) that should contain all users that should have
192-
access to the application by default. This behaviour can also be explicitly controlled by specifying the `spec.allowAllUsers`
193-
field.
191+
If `spec.claims.allowAllUsers` is set to `true`, the group configured by the
192+
`azure.features.groups-assignment.all-users-group-id` will be assigned to the application. This group should contain
193+
all users that should have access to the application by default.
194194

195195
### 1.8 Single-Page Applications
196196

pkg/azure/client/group/group.go

+6-9
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,13 @@ func (g group) getGroups(tx transaction.Transaction) (resource.Resources, error)
7474
return groups, nil
7575
}
7676

77-
noGroupsDefined := tx.Instance.Spec.Claims == nil || len(tx.Instance.Spec.Claims.Groups) == 0
78-
noGroupsLegacyBehaviour := noGroupsDefined && tx.Instance.Spec.AllowAllUsers == nil
79-
allowAllUsersEnabled := tx.Instance.Spec.AllowAllUsers != nil && *tx.Instance.Spec.AllowAllUsers == true
80-
81-
if noGroupsLegacyBehaviour || allowAllUsersEnabled {
82-
allUsersGroups, err := g.getAllUsersGroups(tx)
83-
if err != nil {
84-
return nil, fmt.Errorf("mapping all-users groups to resources: %w", err)
85-
}
77+
allUsersGroups, err := g.getAllUsersGroups(tx)
78+
if err != nil {
79+
return nil, fmt.Errorf("mapping all-users group to resources: %w", err)
80+
}
8681

82+
allowAllUsersEnabled := tx.Instance.Spec.AllowAllUsers != nil && *tx.Instance.Spec.AllowAllUsers == true
83+
if allowAllUsersEnabled && allUsersGroups != nil {
8784
groups.AddAll(allUsersGroups...)
8885
}
8986

0 commit comments

Comments
 (0)