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.

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

ACL mgmt url-decode fix #73

Merged
merged 2 commits into from
Sep 10, 2024
Merged

ACL mgmt url-decode fix #73

merged 2 commits into from
Sep 10, 2024

Conversation

karlcz
Copy link
Contributor

@karlcz karlcz commented Sep 10, 2024

Fixes a regression in the /hatrac/path/name;acl/access/entry resources that allow addressing of individual entries in an access control list.

Since some time during the Flask port and general revision to how URLs are parsed and dispatched, these haven't been working as they need to. The entry value will be URL encoded, e.g. when it is a URI, but this needs to be decoded before it is mapped to the underlying ACL storage in the database. It also needs to be re-encoded when generating a NotFound exception that names the specific ACL entry.

(This is different than namespace and object names which we keep encoded even in the database, and that's where the regression stems from.)

the role is URL-quoted in the API and needs to be unquoted when
compared to ACL arrays in the database. this seems to be broken since
the code was refactored for Flask and other implicit URL unquoting
was removed from the dispatch path.
1. The role must be unquoted when probing the database content
2. The role should be quoted in URLs within NotFound errors
Copy link
Contributor

@mikedarcy mikedarcy left a comment

Choose a reason for hiding this comment

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

👍

@karlcz karlcz merged commit e3d58db into master Sep 10, 2024
1 check failed
@karlcz karlcz deleted the acl_mgmt_urldecode_fix branch September 10, 2024 20:05
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.

2 participants