Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1e6c1e8
fix(security): address various security findings and bugs
Avantol13 Sep 30, 2026
4923024
Merge branch 'master' into fix/security
Avantol13 Sep 30, 2026
bee1ea8
feat(smart): add SMART on FHIR / US Core compliance (CONF-0119, 0120,…
vincentvanhees Oct 5, 2026
d94fbe9
refactor(oidc): make extra-scope allowlist and extra-claims config-dr…
vincentvanhees Oct 6, 2026
2073ea7
Merge branch 'master' into fix/security
Avantol13 Oct 7, 2026
6af61d6
feat(security): address review comments, more tests
Avantol13 Oct 8, 2026
4b46420
fix(passports): gather and validate users for ALL passports in reques…
Avantol13 Oct 8, 2026
c11a06a
Merge pull request #1386 from uc-cdis/fix/security
Avantol13 Oct 9, 2026
80fb694
fix(oidc): validate non-SMART scopes against client allowlist
vincentvanhees Oct 12, 2026
2947eec
test(oidc): add test coverage for SMART/US Core branch and fix two bugs
vincentvanhees Oct 13, 2026
1fe49b4
feat(smart): add SMART on FHIR / US Core compliance (CONF-0119, 0120,…
vincentvanhees Oct 5, 2026
40cbd56
refactor(oidc): make extra-scope allowlist and extra-claims config-dr…
vincentvanhees Oct 6, 2026
90c49f6
fix(oidc): validate non-SMART scopes against client allowlist
vincentvanhees Oct 12, 2026
32eb3b0
test(oidc): add test coverage for SMART/US Core branch and fix two bugs
vincentvanhees Oct 13, 2026
b3b84bb
Merge branch 'feat/uscore-fhir-ig-compliance' of github.com:uc-cdis/f…
vincentvanhees Oct 13, 2026
359899b
fix(oidc): advertise introspection, revocation and private_key_jwt in…
vincentvanhees Oct 13, 2026
0bd6e72
fix: add SMART scopes to SCOPE_DESCRIPTION and graceful fallback for …
vincentvanhees Oct 14, 2026
b61410b
fix: raise session JWT size limit to 8192 to support long FHIR scope …
vincentvanhees Oct 14, 2026
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
51 changes: 51 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Signing keys. `keys/` is gitignored, so a clean checkout never has it, but a
# developer's working tree does and `COPY . /fence` would bake it into the image.
keys/
**/*.pem
**/*.key
jwt-keys.tar

# Deployment secrets and local config
fence-config.yaml
*config*.local.yaml
.env
.netrc

# Local virtualenvs and caches, which would shadow the image's /venv
.venv/
venv/
ENV/
**/__pycache__/
**/*.py[cod]
.mypy_cache/
.pytest_cache/
.cache/
.coverage
.coverage.*
htmlcov/
coverage.xml
.tox/
.hypothesis/

# Build artifacts
build/
dist/
*.egg-info/
.eggs/

# Editor, OS, and agent scratch
.idea/
.vscode/
.DS_Store
.claude/
.serena/

# Compose files and docs, not needed at runtime
docker-compose*.yml
# Root-only on purpose: `fence/static/privacy_policy.md` is served by
# /privacy-policy and must stay in the image, so this must not become `**/*.md`.
*.md
!README.md

# Note: `.git` is deliberately NOT ignored - the builder stage runs
# `git rev-parse HEAD` and `git describe` to generate version_data.py.
6 changes: 3 additions & 3 deletions .secrets.baseline

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions dockerrun.bash
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ if [ -f /fence/jwt-keys.tar ]; then
cd /fence
tar xvf jwt-keys.tar
if [ -d jwt-keys ]; then
# Replace keys/ rather than merging into it. Anything already there came
# from the image build rather than from the mounted secret, and must not
# survive alongside - fence loads every keypair it finds and publishes
# each one in the JWKS.
rm -rf keys
mkdir -p keys
mv jwt-keys/* keys/
fi
Expand Down
16 changes: 0 additions & 16 deletions fence/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,22 +356,6 @@ def logout(next_url, force_era_global_logout=False):
return redirect_response


def check_scope(scope):
def wrapper(f):
@wraps(f)
def check_scope_and_call(*args, **kwargs):
if "_all" in flask.g.scopes or scope in flask.g.scopes:
return f(*args, **kwargs)
else:
raise Unauthorized(
"Requested scope {} can't access this endpoint".format(scope)
)

return check_scope_and_call

return wrapper


def login_required(scope=None):
"""
Create decorator to require a user session
Expand Down
47 changes: 47 additions & 0 deletions fence/blueprints/data/blueprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,14 @@ def upload_data_file():
uploader=uploader,
guid=guid,
)

if guid:
# A supplied GUID can resolve to an existing record instead of creating one
# (See BlankIndex.index_document code), and since
# the signed URL below writes to that record's storage key, we need to
# not just auth the authz provided by the user, but check on the existing record.
_authorize_upload_to_existing_record(blank_index.index_document)

default_expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)

expires_in = get_valid_expiration(
Expand All @@ -202,6 +210,42 @@ def upload_data_file():
return flask.jsonify(response), 201


def _authorize_upload_to_existing_record(index_document: dict) -> None:
"""
Confirm the caller may write to an indexd record that already exists.

Args:
index_document (dict): the indexd record the supplied GUID resolved to

Raises:
Forbidden: if the caller may not write to this record
"""
record_authz = index_document.get("authz")
if record_authz:
authorized = flask.current_app.arborist.auth_request(
jwt=get_jwt(),
service="fence",
methods=["write-storage"],
resources=record_authz,
)
if not authorized:
logger.error(
"Auth error on upload to existing record. User must have "
"'write-storage' access on {}.".format(record_authz)
)
raise Forbidden(
"You do not have write-storage access to this record's authz resources."
)
return

uploader = index_document.get("uploader")
if not uploader or uploader != current_token["context"]["user"]["name"]:
raise Forbidden(
"You cannot upload to this record because it has no authz resources and "
"the uploader field indicates it does not belong to you."
)


@blueprint.route("/multipart/init", methods=["POST"])
@require_auth_header(scope={"data"})
@login_required({"data"})
Expand All @@ -220,6 +264,9 @@ def init_multipart_upload():

blank_index = BlankIndex(file_name=params["file_name"], guid=guid)

if guid:
_authorize_upload_to_existing_record(blank_index.index_document)

default_expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
expires_in = get_valid_expiration(
params.get("expires_in"),
Expand Down
53 changes: 48 additions & 5 deletions fence/blueprints/data/bulk_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,62 @@ def collect_authz_needing_check(guids, index_document, auth_roles):
return authz_needing_check


def _authz_key_resources(authz_key):
"""Return the list of resource paths an authz key stands for."""
return list(authz_key) if isinstance(authz_key, tuple) else [authz_key]


def _is_authorized_for_read_storage(auth_mapping, authz_key, jwt=None, user_id=None):
"""
Determine whether the caller may read-storage every resource in an authz key.

Arborist's ``auth_request`` over the full resource list is the decision, matching
the single-object path: a record carrying several resources requires access to
all of them. The auth mapping only screens out keys with no matching resource at
all, so arborist is not called for keys that cannot pass.

Args:
auth_mapping: Arborist authorization mapping for a user or token.
authz_key: Normalized authz key from indexd metadata.
jwt: Bearer token to authorize, or None when authorizing by user_id or
anonymously.
user_id: Username to authorize, for passport-based access.

Returns:
True if arborist authorizes fence read-storage on every resource in the key.
"""
if not _auth_mapping_has_read_storage(auth_mapping, authz_key):
return False

return bool(
flask.current_app.arborist.auth_request(
jwt=jwt,
user_id=user_id,
service="fence",
methods="read-storage",
resources=_authz_key_resources(authz_key),
)
)


def _auth_mapping_has_read_storage(auth_mapping, authz_key):
"""Determine whether the Arborist auth mapping grants read-storage for an authz key.

This is a pre-filter, not an authorization decision: it passes when any one
resource in the key matches.

Args:
auth_mapping: Arborist authorization mapping for a user or token.
authz_key: Normalized authz key from indexd metadata.

Returns:
True if the auth_mapping contains a fence read-storage permission for the key.
True if the auth_mapping contains a fence read-storage permission for any
resource in the key.
"""
if auth_mapping is None:
return False

authz_list = list(authz_key) if isinstance(authz_key, tuple) else [authz_key]
for resource in authz_list:
for resource in _authz_key_resources(authz_key):
methods_list = auth_mapping.get(resource)
if not methods_list:
continue
Expand Down Expand Up @@ -133,7 +174,9 @@ def build_authz_to_authorized_username(authz_needing_check, users_from_passports
for authz_key in authz_needing_check:
if authz_key in authz_to_authorized_username:
continue
if _auth_mapping_has_read_storage(auth_mapping, authz_key):
if _is_authorized_for_read_storage(
auth_mapping, authz_key, user_id=username
):
authz_to_authorized_username[authz_key] = username
else:
try:
Expand All @@ -148,7 +191,7 @@ def build_authz_to_authorized_username(authz_needing_check, users_from_passports
auth_mapping = {}

for authz_key in authz_needing_check:
if _auth_mapping_has_read_storage(auth_mapping, authz_key):
if _is_authorized_for_read_storage(auth_mapping, authz_key, jwt=token):
authz_to_authorized_username[authz_key] = None

return authz_to_authorized_username
Expand Down
Loading
Loading