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.

Sign up for GitHub

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

core: improve anon/auth token logic #148

Merged
merged 13 commits into from
Sep 24, 2024
Merged

core: improve anon/auth token logic #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
fc44c01
core: TokenAuth request_token fix missing auth
tarilabs Jul 6, 2024
3d434a0
core: use token in auth in subsequent requests
tarilabs Jul 6, 2024
f8409f9
core: if 401 on 2nd attempt, avoid anon tokens
tarilabs Jul 6, 2024
425d035
linting
tarilabs Jul 6, 2024
708027f
guard as headers is Optional
tarilabs Jul 6, 2024
bd07f6c
implement review request
tarilabs Jul 13, 2024
5df68eb
Revert "implement review request"
tarilabs Sep 22, 2024
27ee4c4
Revert "linting"
tarilabs Sep 22, 2024
06f885b
Revert "core: if 401 on 2nd attempt, avoid anon tokens"
tarilabs Sep 22, 2024
05b1e57
Revert "core: TokenAuth request_token fix missing auth" b/c #153
tarilabs Sep 22, 2024
bb4c2ec
implement review comment about anon/req token
tarilabs Sep 23, 2024
9c3f003
linting
tarilabs Sep 23, 2024
7d891e9
review feedback: logging
tarilabs Sep 23, 2024
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
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@ env
__pycache__
.python-version
.venv
.vscode
build
dist
25 changes: 13 additions & 12 deletions oras/auth/token.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,16 @@ def authenticate_request(

h = auth_utils.parse_auth_header(authHeaderRaw)

# First try to request an anonymous token
logger.debug("No Authorization, requesting anonymous token")
anon_token = self.request_anonymous_token(h)
if anon_token:
logger.debug("Successfully obtained anonymous token!")
self.token = anon_token
headers["Authorization"] = "Bearer %s" % self.token
return headers, True

# Next try for logged in token
# if no basic auth, try by request an anonymous token
if not hasattr(self, "_basic_auth"):
anon_token = self.request_anonymous_token(h)
if anon_token:
logger.debug("Successfully obtained anonymous token!")
self.token = anon_token
headers["Authorization"] = "Bearer %s" % self.token
return headers, True

# basic auth is available, try using auth token
token = self.request_token(h)
if token:
self.token = token
Expand All @@ -95,7 +95,7 @@ def authenticate_request(

def request_token(self, h: auth_utils.authHeader) -> bool:
"""
Request an authenticated token and save for later.s
Request an authenticated token and save for later.
"""
params = {}
headers = {}
Expand Down Expand Up @@ -124,6 +124,7 @@ def request_token(self, h: auth_utils.authHeader) -> bool:
# Set Basic Auth to receive token
headers["Authorization"] = "Basic %s" % self._basic_auth

logger.debug(f"Requesting auth token for: {h}")
authResponse = self.session.get(h.realm, headers=headers, params=params) # type: ignore

if authResponse.status_code != 200:
Expand All @@ -150,7 +151,7 @@ def request_anonymous_token(self, h: auth_utils.authHeader) -> bool:
if h.scope:
params["scope"] = h.scope

logger.debug(f"Final params are {params}")
logger.debug(f"Requesting anon token with params: {params}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a sanity check - could this have any parameters we wouldn't want to be printed? I would probably leave this line out entirely.

Copy link
Contributor Author

@tarilabs tarilabs Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

only service and scope:

oras-py/oras/auth/token.py

Lines 147 to 151 in 625c6ff

params = {}
if h.service:
params["service"] = h.service
if h.scope:
params["scope"] = h.scope

which to my understanding are not sensitive info and done analogously for the auth token.

Just to be clear, I've just changed the hardcoded text to make it significant it's about an anon token when observing the logs, and the params was there before :)

I thought aligning log statements in:

to your comment of:

... put this debug message in the function that does that, and put a bit more metadata to make the comment more useful ...

(source)

but let me know if you prefer me to change those log debug statement to something else!

Copy link
Contributor

Choose a reason for hiding this comment

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

only service and scope:

I just wanted to double check this - I think this should be ok to print.

response = self.session.request("GET", h.realm, params=params)
if response.status_code != 200:
logger.debug(f"Response for anon token failed: {response.text}")
Expand Down
3 changes: 3 additions & 0 deletions oras/auth/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ def __init__(self, lookup: dict):
if key in ["realm", "service", "scope"]:
setattr(self, key, lookup[key])

def __repr__(self):
return f"authHeader(lookup={{'service': {repr(self.service)}, 'realm': {repr(self.realm)}, 'scope': {repr(self.scope)}}})"


def parse_auth_header(authHeaderRaw: str) -> authHeader:
"""
Expand Down
4 changes: 3 additions & 1 deletion oras/provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,9 @@ def do_request(
:param stream: stream the responses
:type stream: bool
"""
# Make the request and return to calling function, unless requires auth
# Make the request and return to calling function, but attempt to use auth token if previously obtained
if headers is not None and isinstance(self.auth, oras.auth.TokenAuth):
headers.update(self.auth.get_auth_header())
response = self.session.request(
method,
url,
Expand Down
Loading