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

feat(auth): implement iap auth token #637

Merged
merged 10 commits into from
Sep 22, 2023
Merged

feat(auth): implement iap auth token #637

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9ef32ac
feat(auth): implement iap auth token
jonathan-johnston Sep 14, 2023
3786764
chore(auth): release alpha version for testing
jonathan-johnston Sep 14, 2023
bcabef3
fix(auth): minor tweaks from manual testing
jonathan-johnston Sep 14, 2023
2a77ecf
chore(auth): bump minor version
jonathan-johnston Sep 14, 2023
850a90c
refactor(auth): use BaseToken in unit test
jonathan-johnston Sep 14, 2023
893030c
docs(auth): include IapToken documentation
jonathan-johnston Sep 15, 2023
3e5a225
refactor(auth): use class-level default token ttl
jonathan-johnston Sep 15, 2023
8cfc9a9
refactor(session): remove allow_redirects on get method
jonathan-johnston Sep 15, 2023
fc5d958
refactor(auth): clarify impersonating sa arg name
jonathan-johnston Sep 21, 2023
e7aa915
Merge branch 'master' into jonjon/auth-iap
jonathan-johnston Sep 21, 2023
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
43 changes: 42 additions & 1 deletion auth/gcloud/aio/auth/session.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ async def post(
async def get(
self, url: str, headers: Optional[Mapping[str, str]],
timeout: float, params: Optional[Mapping[str, Union[int, str]]],
stream: bool,
stream: bool, allow_redirects: bool,
) -> Response:
pass

Expand All @@ -79,6 +79,14 @@ async def delete(
) -> Response:
pass

@abstractmethod
async def head(
self, url: str, headers: Optional[Mapping[str, str]],
timeout: float, params: Optional[Mapping[str, Union[int, str]]],
allow_redirects: bool,
) -> Response:
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps https://docs.python.org/3/library/exceptions.html#NotImplementedError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That could definitely work, although I would like to limit the sprawl of my changes and this is an extension of existing form. Since this is an abstract base class and this is an abstract method, you can't instantiate an object of this class so raising is redundant:

>>> import abc
>>> class Base(abc.ABC):
...     def fn(self):
...         return 42
...     @abc.abstractmethod
...     def abstract(self):
...         pass
... 
>>> b = Base()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Can't instantiate abstract class Base with abstract method abstract


@abstractmethod
async def request(
self, method: str, url: str, headers: Mapping[str, str],
Expand Down Expand Up @@ -167,6 +175,7 @@ async def get( # type: ignore[override]
timeout: Timeout = 10,
params: Optional[Mapping[str, Union[int, str]]] = None,
stream: Optional[bool] = None,
allow_redirects: bool = False,
jonathan-johnston marked this conversation as resolved.
Show resolved Hide resolved
) -> aiohttp.ClientResponse:
if stream is not None:
log.warning(
Expand All @@ -177,6 +186,7 @@ async def get( # type: ignore[override]
resp = await self.session.get(
url, headers=headers,
timeout=timeout, params=params,
allow_redirects=allow_redirects,
)
await _raise_for_status(resp)
return resp
Expand Down Expand Up @@ -219,6 +229,21 @@ async def delete( # type: ignore[override]
await _raise_for_status(resp)
return resp

async def head( # type: ignore[override]
self, url: str,
headers: Optional[Mapping[str, str]] = None,
timeout: Timeout = 10,
params: Optional[Mapping[str, Union[int, str]]] = None,
allow_redirects: bool = False,
) -> aiohttp.ClientResponse:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is imported as from aiohttp import ClientResponse as Response. Perhaps change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as the previous comment about wanting to avoid changing unrelated parts of the code.

@TheKevJames might be able to tell you more about this choice.

resp = await self.session.head(
url, headers=headers,
params=params, timeout=timeout,
allow_redirects=allow_redirects,
)
await _raise_for_status(resp)
return resp

async def request( # type: ignore[override]
self, method: str,
url: str, headers: Mapping[str, str],
Expand Down Expand Up @@ -274,11 +299,13 @@ async def get(
timeout: float = 10,
params: Optional[Mapping[str, Union[int, str]]] = None,
stream: bool = False,
allow_redirects: bool = False,
) -> Response:
with self.google_api_lock:
resp = self.session.get(
url, headers=headers, timeout=timeout,
params=params, stream=stream,
allow_redirects=allow_redirects,
)
resp.raise_for_status()
return resp
Expand Down Expand Up @@ -321,6 +348,20 @@ async def delete(
resp.raise_for_status()
return resp

async def head(
self, url: str, headers: Optional[Mapping[str, str]] = None,
timeout: float = 10,
params: Optional[Mapping[str, Union[int, str]]] = None,
allow_redirects: bool = False,
) -> Response:
with self.google_api_lock:
resp = self.session.head(
url, params=params, headers=headers,
timeout=timeout, allow_redirects=allow_redirects,
)
resp.raise_for_status()
return resp

async def request(
self, method: str, url: str, headers: Mapping[str, str],
auto_raise_for_status: bool = True, **kwargs: Any,
Expand Down
Loading