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

Models, routes and views for creating OIDC publishers #10753

Merged
merged 93 commits into from
Apr 5, 2022

Conversation

woodruffw
Copy link
Member

@woodruffw woodruffw commented Feb 14, 2022

This is nowhere near done; just wanted to get something up.

Remaining items:

  • Unit tests
  • Feature-gate these changes (admins only?)
  • Rate limiting

Closes #10617.

@woodruffw woodruffw self-assigned this Feb 14, 2022
warehouse/oidc/models.py Outdated Show resolved Hide resolved
warehouse/oidc/services.py Outdated Show resolved Hide resolved
Comment on lines +22 to +33
class OIDCProviderProjectAssociation(db.Model):
__tablename__ = "oidc_provider_project_association"

oidc_provider_id = Column(
UUID(as_uuid=True),
ForeignKey("oidc_providers.id"),
nullable=False,
primary_key=True,
)
project_id = Column(
UUID(as_uuid=True), ForeignKey("projects.id"), nullable=False, primary_key=True
)
Copy link
Member Author

Choose a reason for hiding this comment

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

N.B.: Let me know if there's a better way to do this many-many mapping. This is the pattern I'm familiar with, but there are probably others.

@woodruffw
Copy link
Member Author

Just pasting some screenshots for visibility.

Separate "publishing" page on each project for OIDC management:

Screen Shot 2022-02-18 at 4 51 38 PM

Flash on successful OIDC provider configuration; removal table:

Screen Shot 2022-02-18 at 4 52 35 PM

Security events for OIDC management:

Screen Shot 2022-02-18 at 4 51 01 PM

@woodruffw woodruffw requested a review from di February 25, 2022 19:45
Copy link
Member

@di di left a comment

Choose a reason for hiding this comment

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

Looks good overall. One broad thought: we probably want an AdminFlag to easily disable this entire feature at once.

warehouse/oidc/forms.py Outdated Show resolved Hide resolved
warehouse/oidc/forms.py Outdated Show resolved Hide resolved
warehouse/templates/manage/publishing.html Outdated Show resolved Hide resolved
warehouse/templates/manage/publishing.html Show resolved Hide resolved
warehouse/templates/manage/publishing.html Outdated Show resolved Hide resolved
warehouse/manage/views.py Outdated Show resolved Hide resolved
warehouse/oidc/forms.py Show resolved Hide resolved
warehouse/oidc/models.py Outdated Show resolved Hide resolved
warehouse/manage/views.py Show resolved Hide resolved
warehouse/manage/views.py Outdated Show resolved Hide resolved
@woodruffw
Copy link
Member Author

Metrics should be fully tagged (where possible) now; tests are updated.

I think the only thing remaining is a response to #10753 (comment); once that's addressed, this should be good to 🚢

@woodruffw
Copy link
Member Author

Disabled job_workflow_ref for now. Should be good to go!

@ewjoachim
Copy link
Contributor

Great job :)

@di di self-requested a review April 1, 2022 21:00
warehouse/config.py Outdated Show resolved Hide resolved
warehouse/manage/views.py Outdated Show resolved Hide resolved
warehouse/manage/views.py Outdated Show resolved Hide resolved
warehouse/email/__init__.py Outdated Show resolved Hide resolved
warehouse/oidc/forms.py Outdated Show resolved Hide resolved
warehouse/oidc/forms.py Outdated Show resolved Hide resolved
warehouse/oidc/forms.py Outdated Show resolved Hide resolved
warehouse/oidc/forms.py Outdated Show resolved Hide resolved
warehouse/templates/manage/publishing.html Outdated Show resolved Hide resolved
@woodruffw woodruffw requested a review from di April 5, 2022 16:11
@woodruffw
Copy link
Member Author

Everything from the last review should be addressed!

@di di merged commit 3c93c03 into pypi:main Apr 5, 2022
@di di deleted the tob-oidc-db-models branch April 5, 2022 17:03
domdfcoding pushed a commit to domdfcoding/warehouse that referenced this pull request Jun 7, 2022
* warehouse/oidc: rough model skeleton

* warehouse/oidc: fix imports

* warehouse/migrations: add migration for OIDC models

* warehouse/migrations: reformat

* warehouse/oidc: add basic verification logic

* oidc/services: reduce clock skew leeway to 30s

* warehouse/oidc: refactor claim verification

* oidc/models: fill in missing properties

* warehouse/migrations: remove original OIDC migration

Add many-many project-provider association.

* warehouse: add OIDC migration, fix association

* warehouse: reformat

* warehouse: OIDC route/view skeleton work

* warehouse: form, view logic for adding OIDC providers

* manage/views: disable HTTP cache, add TODO

* warehouse: move oidc views to "publishing"

...and make it a sub-page for project management.

* warehouse: provider deletion routing

* warehouse: shore up constraints, better error flashes

* warehouse/migrations: rebase revision

* warehouse/templates: update OIDC language

Refer to OIDC providers as "OpenID Connect publishers"

* warehouse: OIDC rate limiting groundwork

* manage/views: clean up OIDC events

* warehouse: use GitHub token for API requests, when available

* oidc/forms: special casing for rate limiting

Record errors with Sentry.

* warehouse: split user/repo form inputs apart

* warehouse/templates: link to GitHub's OIDC docs

* oidc/models: remove actor from checked claims

* templates/email: add OIDC email templates

* warehouse: fix templates, add email sending logic

* warehouse: add an AdminFlag for OIDC control

* oidc/models: use set operators

* oidc/forms: exception driven handling for GitHub API errors

* warehouse: OIDC ratelimiting logic

Also some small HTML fixes.

* warehouse/locale: update translations

* warehouse: lintage

* templates/manage/settings: remove vestigial HTML

* warehouse: address feedback

* Simplify form handling
* Validate GitHub usernames against a regex
* Fix form error presentation

* manage/views: more feedback addressing

* Prevent an infoleak in a session flash
* Reword a confusing comment

* Update warehouse/manage/views.py

Co-authored-by: Dustin Ingram <di@users.noreply.github.com>

* manage/views: fixups

* warehouse: add "OIDC provider removed" emails

* oidc/forms: use GH org regex in callable validator body

* warehouse/locale: update translations

* tests, warehouse: begin writing unit tests

* More tests, restructure for testing

* tests: fill in GitHubProviderForm tests

* tests, warehouse: more tests, adaptations for testing

* tests: more manage/view tests

* tests, warehouse: ratelimit tests, fix bug

* tests: round out ratelimiting

* tests: more tests

* tests, warehouse: OIDC deletion tests

Also, gets some coverage for free by reusing a helper.

* tests, warehouse: fill in model checks

Accommodations for testing.

* oidc/models: type hints

* warehouse/locale: `make translations`

* tests, warehouse: site-wide OIDC feature flag

* warehouse: `make translations`

* treewide: route to 404 when OIDC is disabled

Enable OIDC by default for development environments; update tests.

* warehouse: `make translations`

* Update warehouse/templates/manage/publishing.html

Co-authored-by: Joachim Jablon <ewjoachim@gmail.com>

* oidc/{interfaces,services}: simplify API

* tests: update

* warehouse/migrations: rebase

* tests, warehouse: move ratelimit hit up

* warehouse: `make translations`

* warehouse: plug in more OIDC metrics

Adds additional metrics on:

* Publisher configuration (attempt + ok)
* Publisher removal (attempt + ok)
* JWT signature verification (attempt + ok)

* warehouse/oidc: add a `verify_for_helper` iface method

This encapsulates the entire JWT verification process. It isn't
hooked up to anything yet, but just to get something down.

* manage/views: add provider names to metrics

* oidc/services: add project tag to metrics during JWT verification

* oidc/services: include provider name in metrics too

* tests/unit: plumb metrics through OIDC unit tests

* tests/unit: fill in coverage

* warehouse: `make translations`

* tests, warehouse: disable `job_workflow_ref`

For now.

* Apply suggestions from code review

Co-authored-by: Dustin Ingram <di@users.noreply.github.com>

* tests, warehouse: update tests for changes

Also use `workflow_filename` consistently.

* warehouse, tests: email all users on OIDC changes

Instead of just owners.

* warehouse, tests: include publisher info in OIDC emails

* warehouse: `make translations`

Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
Co-authored-by: Joachim Jablon <ewjoachim@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Warehouse's models to accomodate OIDC verification
4 participants