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

fix: bug fix for token auth #49

Merged
merged 1 commit into from
Oct 21, 2021
Merged

fix: bug fix for token auth #49

merged 1 commit into from
Oct 21, 2021

Conversation

jay-dee7
Copy link
Member

We recently added JWT auth and it introduced a bug that let's anyone
push images under any namespace:

This PR fixes the bug introduced in PR #48

What bug was introduced?

johndoe want to push app1 so he creates an image named openregistry.dev/johndoe/app1, since he's the owner of the image, he can push the image.

johndoe wants to push app1 to janedoe's account, so he makes a request as: openregistry.dev/janedoe/app1 and this worked too (which it should not)

With this PR, we now check that only pull should be allowed and push should be restricted to user's own namespace.

Signed-off-by: jay-dee7 jasdeepsingh.uppal@gmail.com

@jay-dee7 jay-dee7 requested a review from guacamole October 20, 2021 18:57
@jay-dee7 jay-dee7 self-assigned this Oct 20, 2021
@jay-dee7 jay-dee7 force-pushed the token-auth branch 13 times, most recently from a693e29 to 7b142e6 Compare October 21, 2021 06:39
@jay-dee7
Copy link
Member Author

jay-dee7 commented Oct 21, 2021

@guacamole looks like this PR also breaks our github actions. For us to make this work, I had to change the build/run process in actions from docker to Go binary. Maybe we should track this separately? and not include github actions work in this PR?

We recently added JWT auth and it introduced a bug that let's anyone
push images under any namespace:

This PR fixes the bug introduced in PR #48

**johndoe** want to push **app1** so he creates an image named
`openregistry.dev/johndoe/app1`, since he's the owner of the image, he
can push the image.

**johndoe** wants to push **app1** to **janedoe's** account, so he makes
a request as:
`openregistry.dev/janedoe/app1` and this worked too (which it should
not)

With this PR, we now check that only `pull` should be allowed and `push`
should be restricted to user's own namespace.

Signed-off-by: jay-dee7 <jasdeepsingh.uppal@gmail.com>
@jay-dee7
Copy link
Member Author

@guacamole i've reverted the changes to .github/workflows/conformance.yml file for them to be fixed in #50

@guacamole
Copy link
Member

I have opened an issue for same, good to merge 💯

Copy link
Member

@guacamole guacamole left a comment

Choose a reason for hiding this comment

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

LGTM 🥇

@guacamole guacamole merged commit 1911bfc into master Oct 21, 2021
@guacamole guacamole deleted the token-auth branch October 21, 2021 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants