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.

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

openid cleanup #2096

Merged
merged 2 commits into from
Jul 13, 2021
Merged

openid cleanup #2096

merged 2 commits into from
Jul 13, 2021

Conversation

WinnyTroy
Copy link
Contributor

@WinnyTroy WinnyTroy commented May 31, 2021 •
edited
Loading

Changes / Features implemented

  • Add oidc package to base pip requirement file
  • Included necessary configs for oidc package
  • Removed open id connect viewset and tools in favour of oidc library

Steps taken to verify this change does what is intended

  • QA
  • Updated Documentation

Side effects of implementing this change

Closes #2093

@WinnyTroy WinnyTroy force-pushed the 2093_oidc_cleanup branch 2 times, most recently from bfac614 to eec92f8 Compare June 30, 2021 11:49
@WinnyTroy WinnyTroy force-pushed the 2093_oidc_cleanup branch 7 times, most recently from e062860 to a8e89fd Compare July 9, 2021 13:25
DavisRayM
DavisRayM reviewed Jul 12, 2021
View reviewed changes
Copy link
Contributor

@DavisRayM DavisRayM left a comment

Choose a reason for hiding this comment

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

LGTM, left a small comment.

Also wondering if we should make the OIDC package an optional install on this codebase ? Sort of like how we've setup the tableau connector

requirements/base.in Outdated Show resolved Hide resolved
DavisRayM
DavisRayM reviewed Jul 12, 2021
View reviewed changes
Copy link
Contributor

@DavisRayM DavisRayM left a comment

Choose a reason for hiding this comment

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

Minor comment left; Kindly add in some documentation for this feature. A simple link to the repository used and the URLs that can be introduced by this feature.

We should also probably add oidc to the list of blocked usernames. Just in case

@WinnyTroy WinnyTroy force-pushed the 2093_oidc_cleanup branch 3 times, most recently from 0bfb513 to 1e4adf0 Compare July 12, 2021 09:04
DavisRayM
DavisRayM reviewed Jul 12, 2021
View reviewed changes
requirements/base.pip Outdated Show resolved Hide resolved
@WinnyTroy WinnyTroy force-pushed the 2093_oidc_cleanup branch 4 times, most recently from c16654b to 9e6fa2b Compare July 13, 2021 06:28
DavisRayM
DavisRayM reviewed Jul 13, 2021
View reviewed changes
docs/index.rst Outdated Show resolved Hide resolved
@WinnyTroy WinnyTroy force-pushed the 2093_oidc_cleanup branch 3 times, most recently from 1d0fdcc to 1261d01 Compare July 13, 2021 08:03
DavisRayM
DavisRayM previously approved these changes Jul 13, 2021
View reviewed changes
denniswambua
denniswambua previously approved these changes Jul 13, 2021
View reviewed changes
@DavisRayM DavisRayM merged commit be59543 into master Jul 13, 2021
@DavisRayM DavisRayM deleted the 2093_oidc_cleanup branch July 13, 2021 14:14
@DavisRayM DavisRayM mentioned this pull request Jul 27, 2021
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DavisRayM DavisRayM DavisRayM approved these changes

@denniswambua denniswambua denniswambua left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cleanup OIDC
3 participants
@WinnyTroy @denniswambua @DavisRayM