chore(core): remove access pdp internal AttributeInstance type and use policy proto generated struct types instead #471
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sense, and clarify naming
…naming throughout access pdp and authorization service
jakedoublev
changed the base branch from
feat/kas-getdecisions
to
feat/kas-get-decisions
jakedoublev
commented
Mar 28, 2024
| repeated string attribute_value_fqns = 2; | ||
| } | ||
|
|
||
| //A logical bucket of attributes belonging to a "Resource" |
Contributor
Author
There was a problem hiding this comment.
should we unify attribute-fqns in resource attribute also?
jakedoublev
commented
Mar 28, 2024
| assert.Equal(t, mockAttrDefinitions[0], decisions[mockEntityId].Results[0].RuleDefinition) | ||
| } | ||
|
|
||
| // TODO: Is this test accurate? Containing the top AND a lower value results in a fail? |
jakedoublev
commented
Mar 28, 2024
| // fmt.Printf("\nTODO: make access decision here with these fully qualified attributes: %+v\n", attrs) | ||
| // get the entities entitlements | ||
| // | ||
| // TODO: we should already have the subject mappings here and be able to just use OPA to trim down the known data attr values to the ones matched up with the entities |
Contributor
Author
There was a problem hiding this comment.
CC: @pflynn-virtru is this accurate that we can just use the subject mappings & their subject condition sets that we have from the GetAttributesByFqnValues response looking up the data/resource attributes to see which data/resource attributes relate to the subject without retrieving their entitlements separately? That would avoid an extra call to the same GetAttributeByFqnValues endpoint when calling GetEntitlements, right?
jakedoublev
commented
Mar 28, 2024
| Level: logLevel, | ||
| } | ||
| logger := slog.New(slog.NewJSONHandler(os.Stdout, opts)) | ||
| // TODO: uncomment the below when authorization service responds with multiple decisions instead of just a sole permit/deny |
Contributor
Author
There was a problem hiding this comment.
cc @pflynn-virtru: added these test assertions for when the response contains multiple decision responses
jakedoublev
commented
Mar 28, 2024
| assert.NotNil(t, resp) | ||
|
|
||
| // some asserts about resp | ||
| // NOTE: there should be two decision responses, one for each data attribute value, but authorization service |
Contributor
Author
There was a problem hiding this comment.
cc: @pflynn-virtru: Same note here as above that there should be two decisions to assert on when there are multiple resource attribute values, but the service only sends back one at the moment.
pflynn-virtru
added a commit
that referenced
this pull request
Apr 1, 2024
… and use policy proto generated struct types instead (#485) 1. removes `AttributeInstance` type 2. DRY improvements to tests in pdp_test.go 3. adds tests for existing and new helper functions (grouping, FQN relations, etc) 4. fixes commented structure vs real structure of `EntityEntitlements` proto and clarifies field name Addresses #469 Original PR #471 and author @jakedoublev
Member
|
moved to #485 |
sujankota
pushed a commit
that referenced
this pull request
Apr 2, 2024
… and use policy proto generated struct types instead (#485) 1. removes `AttributeInstance` type 2. DRY improvements to tests in pdp_test.go 3. adds tests for existing and new helper functions (grouping, FQN relations, etc) 4. fixes commented structure vs real structure of `EntityEntitlements` proto and clarifies field name Addresses #469 Original PR #471 and author @jakedoublev
tech-guru42
added a commit
to tech-guru42/TDF
that referenced
this pull request
Jun 3, 2024
… and use policy proto generated struct types instead (#485) 1. removes `AttributeInstance` type 2. DRY improvements to tests in pdp_test.go 3. adds tests for existing and new helper functions (grouping, FQN relations, etc) 4. fixes commented structure vs real structure of `EntityEntitlements` proto and clarifies field name Addresses #469 Original PR opentdf/platform#471 and author @jakedoublev
passion-127
added a commit
to passion-127/TDF
that referenced
this pull request
Jun 6, 2024
… and use policy proto generated struct types instead (#485) 1. removes `AttributeInstance` type 2. DRY improvements to tests in pdp_test.go 3. adds tests for existing and new helper functions (grouping, FQN relations, etc) 4. fixes commented structure vs real structure of `EntityEntitlements` proto and clarifies field name Addresses #469 Original PR opentdf/platform#471 and author @jakedoublev
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
AttributeInstancetypeEntityEntitlementsproto and clarifies field nameAddresses #469