-
Notifications
You must be signed in to change notification settings - Fork 24
Scope parsing, classification & policy config #4176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8d0fcac
c7fd6ae
2d16cf7
a0edb15
ba9c58b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,15 +24,46 @@ import ( | |
| "github.com/nuts-foundation/nuts-node/vcr/pe" | ||
| ) | ||
|
|
||
| // ModuleName is the name of the policy module | ||
| // ModuleName is the name of the policy module. | ||
| const ModuleName = "policy" | ||
|
|
||
| // ErrNotFound is returned when no credential profile matches the requested scope. | ||
| var ErrNotFound = errors.New("not found") | ||
|
|
||
| // PDPBackend is the interface for the policy backend | ||
| // Both the remote and local policy backend implement this interface | ||
| // ErrAmbiguousScope is returned when multiple credential profile scopes are found in a single request. | ||
| var ErrAmbiguousScope = errors.New("multiple credential profile scopes found") | ||
|
|
||
| // ScopePolicy defines how extra scopes (beyond the credential profile scope) are handled. | ||
| type ScopePolicy string | ||
|
|
||
| const ( | ||
| // ScopePolicyProfileOnly only accepts the credential profile scope. Extra scopes cause an error. | ||
| ScopePolicyProfileOnly ScopePolicy = "profile-only" | ||
| // ScopePolicyPassthrough grants all requested scopes without evaluation. | ||
| ScopePolicyPassthrough ScopePolicy = "passthrough" | ||
| // ScopePolicyDynamic evaluates extra scopes via an external AuthZen PDP. | ||
| ScopePolicyDynamic ScopePolicy = "dynamic" | ||
| ) | ||
|
|
||
| // CredentialProfileMatch is the result of matching a scope string against the policy configuration. | ||
| // It contains the matched credential profile (WalletOwnerMapping + ScopePolicy) and the | ||
| // remaining scopes that did not match any credential profile. | ||
| type CredentialProfileMatch struct { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i still find this abstraction a bit weird
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair — it is a bit awkward. I landed on this shape because the caller needs four things back (profile scope, mapping, policy, other scopes) and a single struct beats a 5-value return. Happy to restructure if you have a concrete alternative in mind. |
||
| // CredentialProfileScope is the scope that matched a credential profile. | ||
| CredentialProfileScope string | ||
|
Comment on lines
+52
to
+53
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Odd naming, it repeats its container, but just "Scope" might be too ambiguous. Abstraction/naming is off, but I can't think of a better one right now
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed it's imperfect. |
||
| // WalletOwnerMapping contains the PresentationDefinitions per wallet owner type for the matched credential profile. | ||
| WalletOwnerMapping pe.WalletOwnerMapping | ||
| // ScopePolicy is the configured scope policy for the matched credential profile. | ||
| ScopePolicy ScopePolicy | ||
| // OtherScopes contains the scopes from the request that did not match any credential profile. | ||
| OtherScopes []string | ||
| } | ||
|
|
||
| // PDPBackend is the interface for the policy backend. | ||
| // Both the remote and local policy backend implement this interface. | ||
| type PDPBackend interface { | ||
| // PresentationDefinitions returns the PresentationDefinitions (mapped to a WalletOwnerType) for the given scope | ||
| // scopes are space delimited. It's up to the backend to decide how to handle this | ||
| PresentationDefinitions(ctx context.Context, scope string) (pe.WalletOwnerMapping, error) | ||
| // FindCredentialProfile resolves a scope string against the policy configuration. | ||
| // It parses the space-delimited scope string, identifies exactly one credential profile scope, | ||
| // and returns the matched profile along with any remaining scopes. | ||
| FindCredentialProfile(ctx context.Context, scope string) (*CredentialProfileMatch, error) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't it be named "MatchCredentialProfile" if it returns a Match?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Going to keep |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should remove this after the PoCs and "dynamic" is implemented? Because it sounds quite unsafe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss this in #4213 — I think there can be value in a passthrough policy. For now let's keep it in for the PoCs.