Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: allow standard users to get authorization decisions #1634

Merged
merged 4 commits into from
Oct 15, 2024
Merged

fix: allow standard users to get authorization decisions #1634

merged 4 commits into from
Oct 15, 2024

Conversation

mkleene
Copy link
Contributor

@mkleene mkleene commented Oct 14, 2024 •
edited
Loading

Lets standard users retrieve authorization decisions. Does not allow these users to retrieve entitlements. Implements discussion from https://github.com/virtru-corp/data-security-platform/pull/403.

Resolves #1645.

@mkleene mkleene requested a review from a team as a code owner October 14, 2024 13:59
@mkleene mkleene changed the title allow standard users to get authorization decisions fix: allow standard users to get authorization decisions Oct 14, 2024
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@jakedoublev
Copy link
Contributor

Is this a blocker for you @mkleene? There's been recent discussion about adding a Casbin role specific to services that are privileged and distinct from the standard user role to avoid opening up some routes too widely.

@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@mkleene
Copy link
Contributor Author

mkleene commented Oct 14, 2024 •
edited
Loading

Is this a blocker for you @mkleene? There's been recent discussion about adding a Casbin role specific to services that are privileged and distinct from the standard user role to avoid opening up some routes too widely.

@jakedoublev Yes, it is. Gateway's decrypt mode needs to be able to determine if a user has access to content.

@mkleene
Copy link
Contributor Author

mkleene commented Oct 14, 2024 •
edited
Loading

Is this a blocker for you @mkleene? There's been recent discussion about adding a Casbin role specific to services that are privileged and distinct from the standard user role to avoid opening up some routes too widely.

@jakedoublev Although I think that this agrees with @jrschumacher 's thoughts here: https://github.com/virtru-corp/data-security-platform/pull/403, where GetEntitlements is a bit more locked down but these two methods are not.

pflynn-virtru
pflynn-virtru previously approved these changes Oct 15, 2024
View reviewed changes
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
4 similar comments
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@mkleene
Copy link
Contributor Author

mkleene commented Oct 15, 2024

Is this a blocker for you @mkleene? There's been recent discussion about adding a Casbin role specific to services that are privileged and distinct from the standard user role to avoid opening up some routes too widely.

@jakedoublev Although I think that this agrees with @jrschumacher 's thoughts here: virtru-corp/data-security-platform#403, where GetEntitlements is a bit more locked down but these two methods are not.

Implements #1645

@github-actions GitHub Actions
Copy link
Contributor

Warning

This pull request does not reference any issues. Please add a reference to an issue in the body of the pull request description.

jrschumacher
jrschumacher previously approved these changes Oct 15, 2024
View reviewed changes
@mkleene mkleene dismissed stale reviews from jrschumacher and pflynn-virtru via 8489b2d October 15, 2024 15:50
@strantalis strantalis added this pull request to the merge queue Oct 15, 2024
Merged via the queue into main with commit 718f5e3 Oct 15, 2024
20 checks passed
@strantalis strantalis deleted the mkleene-patch-1 branch October 15, 2024 18:34
@opentdf-automation opentdf-automation bot mentioned this pull request Oct 15, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Oct 15, 2024
@opentdf-automation
chore(main): release service 0.4.25 (#1602)
99da3e2 
🤖 I have created a release *beep* *boop*
---


##
[0.4.25](service/v0.4.24...service/v0.4.25)
(2024-10-15)


### Features

* **authz:** Add name to entity id when retrieved from token
([#1616](#1616))
([5304204](5304204))
* **core:** Add entity category to audit logs
([#1614](#1614))
([871878c](871878c))
* **core:** Change log level from Debug to Trace for readiness checks
([#1544](#1544))
([0af1269](0af1269)),
closes [#1545](#1545)
* **policy:** 1004 add audit support for unsafe actions
([#1620](#1620))
([4b64e5b](4b64e5b))
* **policy:** 1357 policy GetAttributeByFqn db query should employ fewer
roundtrips ([#1633](#1633))
([0bdb7e5](0bdb7e5)),
closes [#1357](#1357)
* **policy:** 1421 tech debt migrate Resource Mappings object queries to
sqlc ([#1422](#1422))
([cd74bcf](cd74bcf))
* **policy:** 1426 tech debt migrate Namespace object queries to sqlc -
PART 2 ([#1617](#1617))
([b914350](b914350))
* **policy:** 1434 tech debt migrate attribute value object queries to
sqlc ([#1444](#1444))
([0a7998e](0a7998e)),
closes [#1434](#1434)
* **policy:** 1435 tech debt migrate attribute definition object queries
to sqlc ([#1450](#1450))
([c36624c](c36624c))
* **policy:** 1436 tech debt migrate subject mapping and condition set
object queries to sqlc
([#1606](#1606))
([ec60c9f](ec60c9f))
* **policy:** 1438 tech debt migrate attribute fqn indexing queries to
sqlc ([#1445](#1445))
([617aa91](617aa91)),
closes [#1438](#1438)
* **policy:** 1580 Resource Mappings GET/LIST should provide attribute
value FQNs in response
([#1622](#1622))
([e33bcc0](e33bcc0)),
closes [#1580](#1580)
* **policy:** 1618 update KAS CRUD to align with ADR decisions
([#1619](#1619))
([379f980](379f980)),
closes [#1618](#1618)
* **policy:** DSP-51 - deprecate PublicKey local field
([#1590](#1590))
([e3ed0b5](e3ed0b5))
* **sdk:** Improve KAS key lookup and caching
([#1556](#1556))
([fb6c47a](fb6c47a))


### Bug Fixes

* allow standard users to get authorization decisions
([#1634](#1634))
([718f5e3](718f5e3))
* **authz:** Move logs containing subject mappings to trace level
([#1635](#1635))
([80c117c](80c117c)),
closes [#1503](#1503)
* **core:** Autobump service
([#1611](#1611))
([2567052](2567052))
* **core:** Autobump service
([#1624](#1624))
([9468479](9468479))
* **core:** Autobump service
([#1639](#1639))
([0551247](0551247))
* **core:** Autobump service
([#1654](#1654))
([ecf41e9](ecf41e9))
* **core:** log audit object as json
([#1612](#1612))
([c519ffb](c519ffb))
* Simplify request ID extraction from context for AUDIT
([#1626](#1626))
([2f7518c](2f7518c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrschumacher jrschumacher jrschumacher approved these changes

@pflynn-virtru pflynn-virtru pflynn-virtru left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GetDecisions shouldn't require admin permissions
5 participants
@mkleene @jakedoublev @jrschumacher @pflynn-virtru @strantalis