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

Proof of Concept: Macaroons #2

Closed
wants to merge 4 commits into from
Closed

Proof of Concept: Macaroons #2

wants to merge 4 commits into from

Conversation

dstufft
Copy link
Owner

@dstufft dstufft commented Oct 28, 2018

No description provided.

if dm is None:
raise InvalidMacaroon

verifier = pymacaroons.Verifier()
Copy link
Owner Author

Choose a reason for hiding this comment

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

We would need to implement our verifiers here. We can add arguments to this function like the resource we're operating on or what permission this request is trying to use to pass in values from the outside world, in order to implement something that depends on the current request.

@dstufft
Copy link
Owner Author

dstufft commented Oct 28, 2018

Here's a dump of my NOTES.txt file from when I was working on this proof of concept:

New permission structure, permissions that limit the ability to upload, or to
restrict uploads that create new projects, releases, or files. This particular
permission acts as a tree, where upload grants all, upload:project is similiar,
upload:release can create new releases and files, but not projects, and
upload:file can only upload new files.

upload
  - upload:project
  - upload:release
  - upload:file



Caveat: Permissions = ["upload", "upload:project", "upload:release", "upload:file"]
Caveat: Resources = [Project(foo), Release(1.0), File(foo-1.0.tar.gz, hash=sha256:...)]
Caveat: NotBefore = XXX
Caveat: NotAfter = XXX


- Need to actually make sure we're not relying soley on authentication anywhere,
  and that we're actually checking authentication AND authorization everywhere.


- Identifier Syntax: v:1 user:... id:...


- We probably need to run the verification returns on the given macaroon instead
  of just blindly listening to the inside?

@dstufft dstufft changed the title Macaroons Proof of Concept: Macaroons Oct 28, 2018
except ValueError:
return None

if auth_method.lower() != "macaroon":
Copy link

Choose a reason for hiding this comment

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

I'd expect to see Authorization: Bearer ABCD and another layer dispatch on the format of the token.
Is there a spec for new credential type?

),
sa.Column("last_used", sa.DateTime(), nullable=True),
sa.Column(
"caveats",
Copy link

Choose a reason for hiding this comment

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

What is the rationale of storing root caveats explicitly?
IIRC:

  • root macaroon == f(key, caveats)
  • key must be stored (somewhere) to allow validation
  • id is needed to find the key and to invalidate macaroon tree

Thus, (id, key, root macaroon (bytes) ) is all the information needed, right?
Is the idea to recompute root macaroon on demand?
Can root caveats be edited?
What's validates, caveats in request macaroon or in this table?

Copy link
Owner Author

Choose a reason for hiding this comment

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

The only reason to store the caveats in the database is to make it possible for the UI to show what the maximum scope of this token is. Of course given the way macaroons function we can't tell them what derived macaroons they've made from that initial token and what those are scoped to, but we can tell them what the max scope their token is good for.

We would never use what's in the database here for the validation, it'd just be there for informational purposes.

@dstufft dstufft closed this Jun 30, 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 this pull request may close these issues.

2 participants