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

WIP: don't forget to request the pull scope #1

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions lib/docker/remote/auth_info.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module Docker
module Remote
class AuthInfo
class << self
def from_header(header, creds)
def from_header(header, creds, repo)
idx = header.index(' ')
auth_type = header[0..idx].strip.downcase

Expand All @@ -11,23 +11,24 @@ def from_header(header, creds)
ret[key.strip] = value.strip[1..-2] # remove quotes
end

new(auth_type, params, creds)
new(auth_type, params, creds, repo)
end
end


attr_reader :auth_type, :params, :creds
attr_reader :auth_type, :params, :creds, :repo

def initialize(auth_type, params, creds)
def initialize(auth_type, params, creds, repo)
@auth_type = auth_type
@params = params
@creds = creds
@repo = repo
end

def strategy
@strategy ||= case auth_type
when 'bearer'
BearerAuth.new(self, creds)
BearerAuth.new(self, creds, repo)
when 'basic'
BasicAuth.new(creds)
else
Expand Down
9 changes: 5 additions & 4 deletions lib/docker/remote/bearer_auth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ module Remote
class BearerAuth
include Utils

attr_reader :auth_info, :creds
attr_reader :auth_info, :creds, :repo

def initialize(auth_info, creds)
def initialize(auth_info, creds, repo)
@auth_info = auth_info
@creds = creds
@repo = repo
end

def make_get(path)
Expand All @@ -27,15 +28,15 @@ def realm
end

def service
@serivce ||= auth_info.params['service']
@service ||= auth_info.params['service']
end

def token
@token ||= begin
http = Net::HTTP.new(realm.host, realm.port)
http.use_ssl = true if realm.scheme == 'https'

url_params = { service: service }
url_params = { service: service, scope: "repository:#{repo}:pull" }
Copy link
Member

Choose a reason for hiding this comment

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

The :scope hash entry will get replaced by the body of the if statement below, is that ok? Should the repository:<repo>:pull scope be added to the scopes in www-authenticate or just be a default?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure the function of the body of the if statement below, but it wasn't getting replaced for me. I'm using a Harbor registry and it seemed like the Bearer token was getting requested with no scopes. I used Pixie to inspect the conversation, but I'm not sure I have all the details right, all I could say for sure is I had several processes of different kinds having this conversation (image-reflector-controller and others) and the one that was failing was here.

When I added the repository scope, it no longer received 401 on the request that uses the bearer token (in between building the app image and starting the build for the assets image, where it needs to confirm if any prior asset image exists, it tries to list the tags and fails 401.)

I wouldn't really suggest this change in particular as it's poorly thought out (this token will undoubtedly get used for other stuff besides pulling a specific repo), it was just the quickest way for me to confirm and resolve what was wrong in this one specific conversation that seems to be the only place this gem+token gets used by me, at least for now.


if scope = auth_info.params['scope']
url_params[:scope] = scope
Expand Down
2 changes: 1 addition & 1 deletion lib/docker/remote/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def auth
end

def www_auth(response)
AuthInfo.from_header(response['www-authenticate'], creds)
AuthInfo.from_header(response['www-authenticate'], creds, repo)
end

def get(path, http: registry_http, use_auth: auth, limit: 5)
Expand Down