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

Handle "<token>" as Username from credential helpers. #1490

Closed
coollog opened this issue Feb 19, 2019 · 9 comments
Closed

Handle "<token>" as Username from credential helpers. #1490

coollog opened this issue Feb 19, 2019 · 9 comments

Comments

@coollog
Copy link
Contributor

coollog commented Feb 19, 2019

The Docker Credential helper protocol specifies:

If the secret being stored is an identity token, the Username should be set to <token>.

Jib should consider the case where the username received from a credential helper is <token> and authenticate by using the Secret as an identity token. See Azure/acr-docker-credential-helper#31 (comment) for an example.

Originates from Azure/acr-docker-credential-helper#31 (comment)

@andxu
Copy link
Contributor

andxu commented Feb 22, 2019

@coollog
Copy link
Contributor Author

coollog commented Feb 22, 2019

@andxu thanks! It might be a bit involved and I would recommend outlining your approach here first and splitting the work into multiple subtasks. Let us know if you have any questions!

@andxu
Copy link
Contributor

andxu commented Feb 26, 2019

@coollog Thanks for the advice. I will post it here later today.

@andxu
Copy link
Contributor

andxu commented Feb 26, 2019

First, let's take a look how docker gets the credentials from docker-credential-helper(see inline comments added).

    // https://github.com/docker/cli/blob/master/cli/config/credentials/native_store.go#L115 
    // andy: get crendentials from executing echo "$serverAddress"|docker-credential-helper get  
    creds, err := client.Get(c.programFunc, serverAddress)
	if err != nil {
		...
	}

    // andy: here tokenUsername  = "<token>"
	if creds.Username == tokenUsername {
	    // andy: token auth
		ret.IdentityToken = creds.Secret
	} else {
	    // andy: basic auth
		ret.Password = creds.Secret
		ret.Username = creds.Username
	}

	ret.ServerAddress = serverAddress
// https://github.com/docker/distribution/blob/master/registry/client/auth/session.go#L478
func (th *tokenHandler) fetchToken(params map[string]string, scopes []string) (token string, expiration time.Time, err error) {
     ...
    var refreshToken string

	if th.creds != nil {
		refreshToken = th.creds.RefreshToken(realmURL, service)
	}

	if refreshToken != "" || th.forceOAuth {
	    // andy: token auth
		return th.fetchTokenWithOAuth(realmURL, refreshToken, service, scopes)
	}

    // andy: basic auth
	return th.fetchTokenWithBasicAuth(realmURL, service, scopes)
}

// https://github.com/docker/cli/blob/ae1618713f83e7da07317d579d0675f578de22fa/cli/trust/trust.go#L89
func (scs simpleCredentialStore) RefreshToken(u *url.URL, service string) string {
	return scs.auth.IdentityToken
}

Through above code, in jib, I suggest to change(here takes pull image for example, push is similar):

  1. Add a new field named "refreshToken" in Credential class, when we get the response from docker-credential-helper get, we will check the username, if the username equals <token>, then we will save the password as refresh token, otherwise, we save the username and password.
  2. in BaseImageWithAuthorization#call we should not always get the basic auth token(see code) and then set the registryAuthorization to registryAuthenticator, instead we should set the credentials for registryAuthenticator before authenticatePull.
    // pseudocode for BaseImageWithAuthorization call()

    Credential Credential registryCredential = NonBlockingSteps.get(retrieveBaseRegistryCredentialsStep); = NonBlockingSteps.get(retrieveBaseRegistryCredentialsStep);
    
    registryAuthenticator.setCredential(registryCredential).authenticatePull();                    
  1. in RegistryAuthenticator#authenticate, we should turn into two different ways of retrieving the token.
    // https://github.com/GoogleContainerTools/jib/blob/master/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryAuthenticator.java#L264
    // pseudocode
    private Authorization authenticate(String scope) throws RegistryAuthenticationFailedException {
        ...
        AuthenticationResponseTemplate responseJson = null;
        if (credential != null) {
        	if (StringUtils.isNotBlank(credential.getRefreshToken())) {
        		// example:
			// POST:  https://andyreg.azurecr.io/oauth2/token
			// Body:  grant_type=refresh_token&service=andyreg.azurecr.io&scope=repository:ubuntu:pull&refresh_token=<eyJhbGciOi...88Q>
        		responseJson = fetchTokenWithOAuth(credential, realm, service, scope);
        	} else {
        		// example:
			// GET:  https://auth.docker.io/token?service=registry.docker.io&scope=repository:ubuntu:pull
        		responseJson = fetchTokenWithBasicAuth(credential, realm, service, scope);
        	}
        }
        

        if (responseJson.getToken() == null) {
          throw new RegistryAuthenticationFailedException(
              registryEndpointRequestProperties.getServerUrl(),
              registryEndpointRequestProperties.getImageName(),
              "Did not get token in authentication response from " + authenticationUrl);
        }
        return Authorizations.withBearerToken(responseJson.getToken());
    }               

@coollog
Copy link
Contributor Author

coollog commented Feb 26, 2019

@andxu Thanks for the writeup!

  1. I think we can keep Credential as-is for now and have it just store the username/secret that is retrieved as is (so for refresh tokens, the username would be <token> and the password would be the refresh token). We can consider changing the API of Credential later after this fix is complete (like add a refresh token field or a isRefreshToken method).
  2. That sounds good, but we should probably keep the current behavior for when the username is not <token>.
  3. Looks good - feel free to add the Credential#isRefreshToken method if needed for this.

@andxu
Copy link
Contributor

andxu commented Feb 27, 2019

I am adding test cases, please take a review of my implementation, thanks

@GoogleContainerTools GoogleContainerTools deleted a comment from andxu Feb 27, 2019
@chanseokoh
Copy link
Member

chanseokoh commented Mar 1, 2019

Fixed by #1511. Jib will be able to use ACR through an OAuth2 token retrieved from ACR credential helpers.

@chanseokoh chanseokoh modified the milestone: v1.1.0 Mar 1, 2019
@chanseokoh
Copy link
Member

@andxu v1.0.2 released with the ACR credential helper support. Will work on the doc (#1530) later. Thanks for your awesome contribution!

@andxu
Copy link
Contributor

andxu commented Mar 8, 2019

@coollog @chanseokoh Thank you all for your works to make acr supported.

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

No branches or pull requests

3 participants