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

403 handling #71

Closed
justincormack opened this issue Oct 19, 2020 · 8 comments · Fixed by #115
Closed

403 handling #71

justincormack opened this issue Oct 19, 2020 · 8 comments · Fixed by #115
Assignees
Projects

Comments

@justincormack
Copy link
Member

if you log in with a token not a password, various operations fail with 403. There should probably be a message explaining that your token is not scoped for this operation.

eg token creation is forbidden with a token.

hub-tool token create --description "should fail"
Error: bad status code "403 Forbidden"

account info is also forbidden

something is forbidden in tag ls but it appears to log error and continue ok

hub-tool tag ls justincormack/zfs-kmod 
bad status code "403 Forbidden"TAG                                                                             DIGEST                                                                    STATUS              EXPIRES             LAST UPDATE         LAST PUSHED         LAST PULLED         SIZE
justincormack/zfs-kmod:4.9.105           ...
@ingshtrom
Copy link
Contributor

I thought tokens were global access since we don't actually have any scoping on the tokens (well, I guess RO tokens merged last week, but my OG token was prior to that)?

@silvin-lubecki
Copy link
Collaborator

silvin-lubecki commented Oct 19, 2020

Hmm I wasn't aware of that, I thought tokens had full scope right now... Good to know... I wonder if there's a way to detect the user is using a token as password ?
cc @thaJeztah

@shawnaxsom
Copy link

@silvin-lubecki Tokens without scope have full scope, acting as they have in the past.

Tokens have some restrictions even before scope was introduced. You can't log into the UI, for one. Tokens can't create new JWTs in this test case: https://github.com/docker/saas-mega/blob/cb4b110aacccb4fefee6584add5dde98fb025a49/services/repos-new/system-tests/accounts/users_test.go#L170

For detecting token versus password in Hub, we just check if the password entered looks like a UUID. Maybe you can check for UUID, then if it is a UUID, attempt to use it: https://github.com/docker/saas-mega/blob/cb4b110aacccb4fefee6584add5dde98fb025a49/services/hub-garant/hub/authenticate.go#L114

@thaJeztah
Copy link
Member

Detecting if it's a UUID could work as a "quick fix" (which could be fine for now, as this tool would still be a "preview". Preferably, we should address HUB-3187 for that though.

It may be somewhat tricky still though, as the reason for actions not working could be many (incorrect password, no access to organisation, not the right permissions for the action, or even "not logged in"), many (all?) of those also applying on password authentication. There's also the risk of exposing information; i.e. "non existing" and "non-accessible" private content should be indistinguishable from each other (given; more a concern for images/tags as the existence of organisations and user-accounts are (afaik) public information).

Some other thoughts;

  • Instead of trying to detect the reason, produce a more descriptive error (the provided credentials don't have permissions for this action / no credentials provided)
  • could Docker Hub return a more detailed error, based on the specific conditions? (i.e., "credentials were accepted, but don't have the right scope", which would only be returned if the credentials do have (read) access to the resource, but don't have permissions for the action. If the credentials don't have access to the resource, then return a generic "403" or "404".

@silvin-lubecki silvin-lubecki added this to Backlog in Hub tool Oct 20, 2020
@ingshtrom
Copy link
Contributor

While I agree we need a better error message, this seems like an oversight on the Hub side of things (probably a decision made well before most of us were here) because as things work right now Linux users will not be able to use all of the hub-tool features as you cannot log into the CLI with username + password if you have 2FA turned on

❯ docker login -u ahokanson
Password:
Error response from daemon: Get https://registry-1.docker.io/v2/: unauthorized: please use personal access token to login

Thus we're alienating our Linux users even more by releasing this tool, IMO.

In addition, I would 👍 @thaJeztah as we don't want logic all across our properties checking the login password to see if it is a username or password.

@silvin-lubecki silvin-lubecki moved this from Backlog to Before public release in Hub tool Oct 21, 2020
@chris-crone
Copy link
Member

Arg.. Too many things to fix in this thread :)

@ingshtrom, we should definitely fix this on Linux with something like what aws-okta provides for 2FA. Not sure if @thaJeztah has seen this asked for on docker/cli already?

@thaJeztah, it would be great if we could improve the Hub error descriptions.

For getting this tool out into the world, I'd suggest that on getting a 403 with a password structured as a UUID, we catch this in the CLI and output:

<original error>
Personal access tokens are not able to perform all Docker Hub operations, login with a username and password to use this functionality

@silvin-lubecki
Copy link
Collaborator

@chris-crone actually the backend returns this:

{
    "message": "access to the resource is forbidden with personal access token"
}

@chris-crone
Copy link
Member

@silvin-lubecki ha, so that makes it a lot easier :)

@chris-crone chris-crone self-assigned this Nov 12, 2020
@chris-crone chris-crone moved this from Before public release to In progress in Hub tool Nov 12, 2020
@chris-crone chris-crone moved this from In progress to Done in Hub tool Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

6 participants