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

DM-41998: Add support for per-user ingresses #911

Merged
merged 3 commits into from
Dec 4, 2023
Merged
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
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