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

Refactor for_agent / clear up API usage #623

Closed
joepio opened this issue Mar 21, 2023 · 1 comment · Fixed by #624
Closed

Refactor for_agent / clear up API usage #623

joepio opened this issue Mar 21, 2023 · 1 comment · Fixed by #624

Comments

@joepio
Copy link
Member

joepio commented Mar 21, 2023

Authorization in atomic_lib is done by passing a for_agent to much of the get_x type functions. When None is passed, we don't perform any rights checks. This behavior might be confusing to new users of atomic_lib, as they might assume that None means the public agent should be checked.

I think we have two options to improve this:

Use one type, improve documentation

Add a ForAgent type, and explain carefully how that works (see above).

We still have the issue that an atomic_lib user might screw up and leak data if they accidentally pass None while they mean public_agent.

Use a URL that represents sudo agent

Similar to urls::PUBLIC_AGENT, we add a urls::SUDO_AGENT. Whenever an atomic_lib user wants to perform some query, they can use this URL.

Unfortunately, this seems a lot more verbose and annoying to use.

Create an enum ForAgent

Is structured, clear... Seems like the way to go!

joepio added a commit that referenced this issue Mar 21, 2023
@joepio joepio mentioned this issue Mar 21, 2023
4 tasks
@joepio
Copy link
Member Author

joepio commented Mar 21, 2023

I'm implementing the new ForAgent enum, and as I'm doing that, I'm trying to change some defaults. For example, I don't want Query::default to do powerful Sudo queries, but instead, default to Public queries.

Unfortunately, this broke one of the tests. db::test::destroy_resource_and_check_collection_and_commits Now fails, because it returns 2 instead of 1 Agents in one of the first steps. Turns out that the init command is actually fetching an agent from AtomicData.dev, probably during populate_collection, due to a rights check...

It turned out that because I set the Query::default to have ForAgent::public, it now actually performed rights checks, even during populate, which meant that when it tried to make the Class collections, it checked if public_agent had read rights for all of these Classes, which meant it had to check all parents, which for one Class (the Error class) was an actual agent. And THAT bumped the amount of Agents with one.

joepio added a commit that referenced this issue Mar 21, 2023
joepio added a commit that referenced this issue Mar 22, 2023
joepio added a commit that referenced this issue Mar 29, 2023
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 a pull request may close this issue.

1 participant