Skip to content

Chore/add hide idps #1304

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

Jump to bottom
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Chore/add hide idps #1304

wants to merge 6 commits into from
+73 −0

Conversation

george42-ctds
Copy link
Contributor

@george42-ctds george42-ctds commented Oct 13, 2025 •
edited by atlassian bot
Loading

JIRA ticket: HP-2270

New Features

  • Allow a HIDE_IDPS configuration option when shib_ipds=*

Breaking Changes

Bug Fixes

Improvements

Dependency updates

Deployment changes

@coveralls
Copy link

Pull Request Test Coverage Report for Build 18479278440

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 37 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.009%) to 74.889%
Files with Coverage Reduction New Missed Lines %
config.py 16 67.14%
blueprints/login/init.py 21 87.04%
Totals Coverage Status
Change from base Build 18204386563: 0.009%
Covered Lines: 8413
Relevant Lines: 11234
💛 - Coveralls

@github-actions GitHub Actions
Copy link

filepath passed skipped SUBTOTAL
tests/test_oauth2.py 15 0 15
tests/test_centralized_auth.py 16 0 16
tests/test_audit_service.py 3 3 6
tests/test_data_upload.py 8 1 9
tests/test_presigned_url.py 7 0 7
tests/test_dbgap.py 4 1 5
tests/test_drs_endpoint.py 4 0 4
tests/test_oidc_client.py 2 0 2
tests/test_user_token.py 5 0 5
tests/test_google_data_access.py 1 0 1
tests/test_client_credentials.py 1 0 1
tests/test_ras_authn.py 0 3 3
tests/test_register_user.py 0 2 2
TOTAL 66 10 76

Please find the detailed integration test report here

Please find the Github Action logs here

mfshao
mfshao reviewed Oct 21, 2025
View reviewed changes
fence/blueprints/login/__init__.py
upstream_idps = UPSTREAM_IDP_CACHE.get(cache_key)
if hide_idps_list and len(hide_idps_list) > 0:
hidden_idps_excluded = [
x for x in upstream_idps if x.get("name") not in hide_idps_list
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the idea is right. But I also think we should be putting the IDP's entityID into HIDE_IDPS instead of their names. Since this is how Shibboleth IDPs can be configured to enable in Fence config now, like

fence/docs/additional_documentation/fence_shibboleth.md

Lines 84 to 86 in 2a0ede3

shib_idps:
- urn:mace:incommon:nih.gov
- urn:mace:incommon:uchicago.edu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mfshao mfshao mfshao left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@george42-ctds @coveralls @mfshao