Skip to content

Commit

Permalink
Merge pull request #911 from lsst-sqre/tickets/DM-41998
Browse files Browse the repository at this point in the history
DM-41998: Add support for per-user ingresses
  • Loading branch information
rra authored Dec 4, 2023
2 parents c68b204 + a10c46f commit 9aa91d7
Show file tree
Hide file tree
Showing 15 changed files with 188 additions and 72 deletions.
6 changes: 1 addition & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ repos:
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix]

- repo: https://github.com/psf/black
rev: 23.11.0
hooks:
- id: black
- id: ruff-format

- repo: https://github.com/adamchainz/blacken-docs
rev: 1.16.0
Expand Down
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ help:
# npm dependencies have to be installed for pre-commit eslint to work.
.PHONY: init
init:
pip install --upgrade pip
pip install --upgrade pre-commit tox tox-docker docker
pip install --editable .
pip install --upgrade -r requirements/main.txt -r requirements/dev.txt
rm -rf .tox
pip install --upgrade tox tox-docker docker
pre-commit install
cd ui && npm install --legacy-peer-deps

Expand Down
3 changes: 3 additions & 0 deletions changelog.d/20231204_142753_rra_DM_41998.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### New features

- An ingress may now be restricted to a specific user by setting the `username` attribute in the `config` section of a `GafaelfawrIngress`, or the corresponding `username` query parameter to the `/auth` route. Any other user will receive a 403 error. The scope requiremments must also still be met.
3 changes: 3 additions & 0 deletions changelog.d/20231204_144736_rra_DM_41998.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Bug fixes

- Add an ARIA label to the icon for deleting a token in the user interface for better accessibility.
7 changes: 7 additions & 0 deletions crds/ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,13 @@ spec:
- true
required:
- anonymous
username:
type: string
description: >-
Restrict access to this ingress to the given username. All
other users, regardless of their scopes, will receive 403
errors. The user's token must still satisfy any scope
constraints.
template:
type: object
description: "The template used to create the ingress."
Expand Down
13 changes: 13 additions & 0 deletions docs/user-guide/gafaelfawringress.rst
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,19 @@ The same token will also still be passed in the ``X-Auth-Request-Token`` header.

If this configuration option is set, the incoming ``Authorization`` header will be entirely replaced by one containing only the delegated token, unlike Gafaelfawr's normal behavior of preserving any incoming ``Authorization`` header that doesn't include a Gafaelfawr token.

Per-user ingresses
==================

Access to an ingress may be restricted to a specific user as follows:

.. code-block:: yaml
config:
username: <username>
Any user other than the one with the username ``<username>`` will then receive a 403 error.
The scope requirements must still be met to allow access.

.. _anonymous:

Anonymous ingresses
Expand Down
5 changes: 5 additions & 0 deletions docs/user-guide/manual-ingress.rst
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,9 @@ Most but not all of these are discussed above.
If set to a true value, replace the ``Authorization`` header with one containing the delegated token as a bearer token.
This option only makes sense in combination with ``notebook`` or ``delegate_to``.

``username`` (optional)
If set, access to this ingress is restricted to the specified user.
Any other user will receive a 403 error.
``scope`` must still be set and its requirements are still enforced.

These parameters must be URL-encoded as GET parameters to the ``/auth`` route.
28 changes: 25 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,11 @@ python_files = [
# with noqa markers. This is therefore a reiatively relaxed configuration that
# errs on the side of disabling legitimate lints.
#
# Reference for settings: https://beta.ruff.rs/docs/settings/
# Reference for rules: https://beta.ruff.rs/docs/rules/
# Reference for settings: https://docs.astral.sh/ruff/settings/
# Reference for rules: https://docs.astral.sh/ruff/rules/
[tool.ruff]
exclude = [
"docs/**",
"docs/conf.py",
]
line-length = 79
ignore = [
Expand All @@ -166,6 +166,7 @@ ignore = [
"PLR0911", # often many returns is clearer and simpler style
"PLR0913", # factory pattern uses constructors with many arguments
"PLR2004", # too aggressive about magic values
"PLW0603", # yes global is discouraged but if needed, it's needed
"S105", # good idea but too many false positives on non-passwords
"S106", # good idea but too many false positives on non-passwords
"S107", # good idea but too many false positives on non-passwords
Expand All @@ -179,6 +180,24 @@ ignore = [
"TD003", # we don't require issues be created for TODOs
"TID252", # if we're going to use relative imports, use them always
"TRY003", # good general advice but lint is way too aggressive
"TRY301", # sometimes raising exceptions inside try is the best flow

# The following settings should be disabled when using ruff format
# per https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules
"W191",
"E111",
"E114",
"E117",
"D206",
"D300",
"Q000",
"Q001",
"Q002",
"Q003",
"COM812",
"COM819",
"ISC001",
"ISC002",
]
select = ["ALL"]
target-version = "py311"
Expand All @@ -187,6 +206,9 @@ target-version = "py311"
"src/gafaelfawr/handlers/**" = [
"D103", # FastAPI handlers should not have docstrings
]
"docs/**" = [
"INP001", # Python files are embedded diagrams
]
"tests/**" = [
"C901", # tests are allowed to be complex, sometimes that's convenient
"D101", # tests don't need docstrings
Expand Down
2 changes: 0 additions & 2 deletions requirements/dev.in
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,11 @@
asgi-lifespan
coverage[toml]
mypy
pre-commit
pytest
pytest-asyncio
pytest-cov
pytest-sugar
respx
ruff
selenium-wire
sqlalchemy[mypy]
types-cachetools
Expand Down
60 changes: 0 additions & 60 deletions requirements/dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,6 @@ cffi==1.16.0 \
# via
# -c requirements/main.txt
# cryptography
cfgv==3.4.0 \
--hash=sha256:b7265b1f29fd3316bfcd2b330d63d024f2bfd8bcb8b0272f8e19a504856c48f9 \
--hash=sha256:e52591d4c5f5dead8e0f673fb16db7949d2cfb3f7da4582893288f0ded8fe560
# via pre-commit
charset-normalizer==3.3.2 \
--hash=sha256:06435b539f889b1f6f4ac1758871aae42dc3a8c0e24ac9e60c2384973ad73027 \
--hash=sha256:06a81e93cd441c56a9b65d8e1d043daeb97a3d0856d177d5c90ba85acb3db087 \
Expand Down Expand Up @@ -399,10 +395,6 @@ diagrams==0.23.4 \
--hash=sha256:1ba69d98fcf8d768dbddf07d2c77aba6cc95c2e6f90f37146c04c96bc6765450 \
--hash=sha256:b7ada0b119b5189dd021b1dc1467fad3704737452bb18b1e06d05e4d1fa48ed7
# via sphinx-diagrams
distlib==0.3.7 \
--hash=sha256:2e24928bc811348f0feb63014e97aaae3037f2cf48712d51ae61df7fd6075057 \
--hash=sha256:9dafe54b34a028eafd95039d5e5d4851a13734540f1331060d31c9916e7147a8
# via virtualenv
documenteer[guide]==1.0.0a15 \
--hash=sha256:9590ba12c6aca7f76faef5605070059113b1d0a801875f42e69444848c3746ec \
--hash=sha256:caa258ec5f5f68dca976e56098fa0d8a15974566ecca1df6962419ea27063c27
Expand All @@ -422,10 +414,6 @@ docutils==0.20.1 \
# sphinx-jinja
# sphinx-prompt
# sphinxcontrib-bibtex
filelock==3.13.1 \
--hash=sha256:521f5f56c50f8426f5e03ad3b281b490a87ef15bc6c526f168290f0c7148d44e \
--hash=sha256:57dbda9b35157b05fb3e58ee91448612eb674172fab98ee235ccb0b5bee19a1c
# via virtualenv
gitdb==4.0.11 \
--hash=sha256:81a3407ddd2ee8df444cbacea00e2d038e40150acfa3001696fe0dcf1d3adfa4 \
--hash=sha256:bf5421126136d6d0af55bc1e7c1af1c397a34f5b7bd79e776cd3e89785c2b04b
Expand Down Expand Up @@ -536,10 +524,6 @@ hyperframe==6.0.1 \
# via
# h2
# selenium-wire
identify==2.5.32 \
--hash=sha256:0b7656ef6cba81664b783352c73f8c24b39cf82f926f78f4550eda928e5e0545 \
--hash=sha256:5d9979348ec1a21c768ae07e0a652924538e8bce67313a73cb0f681cf08ba407
# via pre-commit
idna==3.6 \
--hash=sha256:9ecdbbd083b06798ae1e86adcbfe8ab1479cf864e4ee30fe4e46a003d12491ca \
--hash=sha256:c05567e9c24a6b9faaa835c4821bad0590fbb9d5779e7caa6e1cc4978e7eb24f
Expand Down Expand Up @@ -707,10 +691,6 @@ myst-parser==2.0.0 \
--hash=sha256:7c36344ae39c8e740dad7fdabf5aa6fc4897a813083c6cc9990044eb93656b14 \
--hash=sha256:ea929a67a6a0b1683cdbe19b8d2e724cd7643f8aa3e7bb18dd65beac3483bead
# via documenteer
nodeenv==1.8.0 \
--hash=sha256:d51e0c37e64fbf47d017feac3145cdbb58836d7eee8c6f6d3b6880c5456227d2 \
--hash=sha256:df865724bb3c3adc86b3876fa209771517b0cfe596beff01a92700e0e8be4cec
# via pre-commit
outcome==1.3.0.post0 \
--hash=sha256:9dcf02e65f2971b80047b377468e72a268e15c0af3cf1238e6ff14f7f91143b8 \
--hash=sha256:e771c5ce06d1415e356078d3bdd68523f284b4ce5419828922b6871e65eda82b
Expand All @@ -723,18 +703,10 @@ packaging==23.2 \
# pytest
# pytest-sugar
# sphinx
platformdirs==4.1.0 \
--hash=sha256:11c8f37bcca40db96d8144522d925583bdb7a31f7b0e37e3ed4318400a8e2380 \
--hash=sha256:906d548203468492d432bcb294d4bc2fff751bf84971fbb2c10918cc206ee420
# via virtualenv
pluggy==1.3.0 \
--hash=sha256:cf61ae8f126ac6f7c451172cf30e3e43d3ca77615509771b3a984a0730651e12 \
--hash=sha256:d89c696a773f8bd377d18e5ecda92b7a3793cbe66c87060a6fb58c7b6e1061f7
# via pytest
pre-commit==3.5.0 \
--hash=sha256:5804465c675b659b0862f07907f96295d490822a450c4c40e747d0b1c6ebcb32 \
--hash=sha256:841dc9aef25daba9a0238cd27984041fa0467b4199fc4852e27950664919f660
# via -r requirements/dev.in
pyasn1==0.5.1 \
--hash=sha256:4439847c58d40b1d0a573d07e3856e95333f1976294494c325775aeca506eb58 \
--hash=sha256:6d391a96e59b23130a5cfa74d6fd7f388dbbe26cc8f1edf39fdddf08d9d6676c
Expand Down Expand Up @@ -985,7 +957,6 @@ pyyaml==6.0.1 \
# -c requirements/main.txt
# documenteer
# myst-parser
# pre-commit
# pybtex
# sphinxcontrib-redoc
referencing==0.31.1 \
Expand Down Expand Up @@ -1109,25 +1080,6 @@ rpds-py==0.13.2 \
# via
# jsonschema
# referencing
ruff==0.1.6 \
--hash=sha256:03910e81df0d8db0e30050725a5802441c2022ea3ae4fe0609b76081731accbc \
--hash=sha256:05991ee20d4ac4bb78385360c684e4b417edd971030ab12a4fbd075ff535050e \
--hash=sha256:137852105586dcbf80c1717facb6781555c4e99f520c9c827bd414fac67ddfb6 \
--hash=sha256:1610e14750826dfc207ccbcdd7331b6bd285607d4181df9c1c6ae26646d6848a \
--hash=sha256:1b09f29b16c6ead5ea6b097ef2764b42372aebe363722f1605ecbcd2b9207184 \
--hash=sha256:1cf5f701062e294f2167e66d11b092bba7af6a057668ed618a9253e1e90cfd76 \
--hash=sha256:3a0cd909d25f227ac5c36d4e7e681577275fb74ba3b11d288aff7ec47e3ae745 \
--hash=sha256:4558b3e178145491e9bc3b2ee3c4b42f19d19384eaa5c59d10acf6e8f8b57e33 \
--hash=sha256:491262006e92f825b145cd1e52948073c56560243b55fb3b4ecb142f6f0e9543 \
--hash=sha256:5c549ed437680b6105a1299d2cd30e4964211606eeb48a0ff7a93ef70b902248 \
--hash=sha256:683aa5bdda5a48cb8266fcde8eea2a6af4e5700a392c56ea5fb5f0d4bfdc0240 \
--hash=sha256:87455a0c1f739b3c069e2f4c43b66479a54dea0276dd5d4d67b091265f6fd1dc \
--hash=sha256:88b8cdf6abf98130991cbc9f6438f35f6e8d41a02622cc5ee130a02a0ed28703 \
--hash=sha256:bd98138a98d48a1c36c394fd6b84cd943ac92a08278aa8ac8c0fdefcf7138f35 \
--hash=sha256:e8fd1c62a47aa88a02707b5dd20c5ff20d035d634aa74826b42a1da77861b5ff \
--hash=sha256:ea284789861b8b5ca9d5443591a92a397ac183d4351882ab52f6296b4fdd5462 \
--hash=sha256:fd89b45d374935829134a082617954120d7a1470a9f0ec0e7f3ead983edc48cc
# via -r requirements/dev.in
scriv[toml]==1.5.0 \
--hash=sha256:2d28eb930dc16ad9499efa0e5f10ddfb98440ab553783426930ff98b0b391917 \
--hash=sha256:f8e4d61439c7085d9bc5053c7fb0df50b606d5c03db4e642b0d44f74aa1b9ecf
Expand Down Expand Up @@ -1418,10 +1370,6 @@ urllib3[socks]==2.1.0 \
# documenteer
# requests
# selenium
virtualenv==20.25.0 \
--hash=sha256:4238949c5ffe6876362d9c0180fc6c3a824a7b12b80604eeb8085f2ed7460de3 \
--hash=sha256:bf51c0d9c7dd63ea8e44086fa1e4fb1093a31e963b86959257378aef020e1f1b
# via pre-commit
wsproto==1.2.0 \
--hash=sha256:ad565f26ecb92588a3e43bc3d96164de84cd9902482b130d0ddbaa9664a85065 \
--hash=sha256:b9acddd652b585d75b20477888c56642fdade28bdfd3579aa24a4d2c037dd736
Expand Down Expand Up @@ -1476,11 +1424,3 @@ zstandard==0.22.0 \
--hash=sha256:f9b2cde1cd1b2a10246dbc143ba49d942d14fb3d2b4bccf4618d475c65464912 \
--hash=sha256:fe3390c538f12437b859d815040763abc728955a52ca6ff9c5d4ac707c4ad98e
# via selenium-wire

# The following packages are considered to be unsafe in a requirements file:
setuptools==69.0.2 \
--hash=sha256:1e8fdff6797d3865f37397be788a4e3cba233608e9b509382a2777d25ebde7f2 \
--hash=sha256:735896e78a4742605974de002ac60562d286fa8051a7e2299445e8e8fbb01aa6
# via
# -c requirements/main.txt
# nodeenv
27 changes: 27 additions & 0 deletions src/gafaelfawr/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ class AuthConfig:
use_authorization: bool
"""Whether to put any delegated token in the ``Authorization`` header."""

username: str | None
"""Restrict access to the ingress to only this username."""


def auth_uri(
x_original_uri: (str | None) = Header(
Expand Down Expand Up @@ -152,6 +155,17 @@ def auth_config(
),
examples=[True],
),
username: str | None = Query(
None,
title="Restrict to username",
description=(
"Only allow access to this ingress by the user with the given"
" username. All other users, regardless of scopes, will receive"
" 403 errors. The user must still meet the scope requirements"
" for the ingress."
),
examples=["rra"],
),
auth_uri: str = Depends(auth_uri),
context: RequestContext = Depends(context_dependency),
) -> AuthConfig:
Expand All @@ -174,6 +188,8 @@ def auth_config(
required_scopes=sorted(scopes),
satisfy=satisfy.name.lower(),
)
if username:
context.rebind_logger(required_user=username)

if delegate_scope:
delegate_scopes = {s.strip() for s in delegate_scope.split(",")}
Expand All @@ -193,6 +209,7 @@ def auth_config(
delegate_scopes=delegate_scopes,
minimum_lifetime=lifetime,
use_authorization=use_authorization,
username=username,
)


Expand Down Expand Up @@ -308,6 +325,16 @@ async def get_auth(
auth_config.scopes,
)

# Check a user constraint. InsufficientScopeError is not really correct,
# but none of the RFC 6750 error codes are correct and it's the closest.
if auth_config.username and token_data.username != auth_config.username:
raise generate_challenge(
context,
auth_config.auth_type,
InsufficientScopeError("Access not allowed for this user"),
auth_config.scopes,
)

# Log and return the results.
context.logger.info("Token authorized")
headers = await build_success_headers(context, auth_config, token_data)
Expand Down
13 changes: 12 additions & 1 deletion src/gafaelfawr/models/kubernetes.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,9 @@ class GafaelfawrIngressConfig(CamelCaseModel):
)
"""The scopes to require for access."""

username: str | None = None
"""Restrict access to the given user."""

@model_validator(mode="after")
def _validate_conflicts(self) -> Self:
"""Check for conflicts between settings.
Expand All @@ -288,7 +291,13 @@ def _validate_conflicts(self) -> Self:
raise ValueError(msg)

if self.scopes and self.scopes.is_anonymous():
fields = ("auth_type", "delegate", "login_redirect", "replace_403")
fields = (
"auth_type",
"delegate",
"login_redirect",
"replace_403",
"username",
)
for snake_name in fields:
if getattr(self, snake_name, None):
camel_name = to_camel_case(snake_name)
Expand Down Expand Up @@ -326,6 +335,8 @@ def to_auth_url(self) -> str:
query.append(("use_authorization", "true"))
if self.auth_type:
query.append(("auth_type", self.auth_type.value))
if self.username:
query.append(("username", self.username))
return f"{base_url}/auth?" + urlencode(query)


Expand Down
Loading

0 comments on commit 9aa91d7

Please sign in to comment.