Skip to content

Add SP OBO Graph overage claim resolution support - #20

Open
hamm-bit wants to merge 11 commits into
Azure:masterfrom
hamm-bit:add-sp-overage-resolution
Open

Add SP OBO Graph overage claim resolution support#20
hamm-bit wants to merge 11 commits into
Azure:masterfrom
hamm-bit:add-sp-overage-resolution

Conversation

@hamm-bit

Copy link
Copy Markdown

Currently guard only supports user principals overage resolution and places a hard block on SP overage. This commit adds support for Service Principals On-Behalf-Of flow.

Comment thread auth/providers/azure/graph/graph.go Outdated
Comment thread auth/providers/azure/graph/graph.go Outdated
Comment thread auth/providers/azure/azure.go Outdated
Comment thread auth/providers/azure/graph/graph.go Outdated
Comment thread auth/providers/azure/graph/graph.go Outdated
Comment thread auth/providers/azure/graph/graph.go
Comment thread auth/providers/azure/graph/graph.go Outdated
Comment thread auth/providers/azure/graph/graph.go Outdated
Comment thread auth/providers/azure/azure.go
Comment thread auth/providers/azure/graph/graph.go
Comment thread auth/providers/azure/graph/graph.go
Comment thread auth/providers/azure/azure.go Outdated
Comment thread auth/providers/azure/graph/graph.go Outdated
Comment thread auth/providers/azure/azure_test.go Outdated
@karataliu

Dong Liu (karataliu) commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

per discussion, please impl different path for user and sp in akstokenprovider side #Closed

@hamm-bit
hamm-bit force-pushed the add-sp-overage-resolution branch from 0c97fb1 to c51043e Compare August 20, 2026 10:51
Comment thread auth/providers/azure/graph/graph.go Outdated
Comment thread auth/providers/azure/azure.go Outdated
Comment thread auth/providers/azure/graph/graph.go Outdated
Comment thread auth/providers/azure/graph/graph.go Outdated
Comment thread auth/providers/azure/azure.go Outdated
Comment thread auth/providers/azure/graph/graph.go Outdated
// transport error and the message is discarded. We need this error
// to surface in API server logs so operators can diagnose why the
// SPN authentication was rejected.
if isAppToken(claims) {

@karataliu Dong Liu (karataliu) Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of removing it, can add a flag to support sp, and use that flag to control the logic?
later the flag can be removed #Closed

Comment thread auth/providers/azure/options.go Outdated
Comment thread auth/providers/azure/options.go Outdated
Comment thread auth/providers/azure/azure.go Outdated
}
resp.Groups, err = s.graphClient.GetGroups(ctx, resp.Username, token)
principal := resp.Username
isServicePrincipal := s.Options.EnableSPGroupResolution && isAppToken(claims)

@karataliu Dong Liu (karataliu) Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isServicePrincipal := isAppToken(claims) ? makes more sense?
it could be computed once, and reused later for isAppToken check

also by here, no need to check EnableSPGroupResolution again? #Closed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logically if ResolveGroupMembershipOnlyOnOverageClaim true, EnableSPGroupResolution already checked
if false, it no harm to do real sp path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants