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

Authorize Generic user for Streamlit #3672

Merged
merged 5 commits into from
Oct 30, 2024
Merged

Conversation

dfitchett
Copy link
Contributor

What was the problem?

Currently, anyone with access to the deployed url via CAG would be able access the application. There is no authentication or authorization protecting any part of the application.

Associated tickets or Slack threads:

How does this fix it?1

This adds a level of authentication via github OAuth apps. Anyone with a github account will be able to access the application as a basic user. Eventually more authorization will be added to only allow people who are part of the department-of-veterans-affairs organization and part of specific teams.

How to test this PR

  1. Pull branch
  2. from root dir, run ./gradlew vro-streamlit:check to run all checks/lints/unit tests.
  3. from root dir, run ./gradlew :vro-streamlit:docker then run COMPOSE_PROFILES="streamlit" ./gradlew :dockerComposeUp to run the container. Then open browser to http://0.0.0.0:8501/ to view webpages.
  4. On the Home page, click the login with github and authorize to view the BIE Events pages.

Footnotes

  1. Pull-Requests guidelines. If PR is significant, update Current Software State wiki page.

@dfitchett dfitchett requested a review from a team as a code owner October 29, 2024 19:17
@dfitchett dfitchett requested review from Ponnia-M and brostk October 29, 2024 19:17
Copy link
Contributor

github-actions bot commented Oct 29, 2024

Test Results

136 tests  ±0   136 ✅ ±0   27s ⏱️ +3s
 39 suites ±0     0 💤 ±0 
 39 files   ±0     0 ❌ ±0 

Results for commit b227f7c. ± Comparison against base commit 754003a.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Oct 29, 2024

JaCoCo Test Coverage

Overall Project 72.23%

There is no coverage information present for the Files changed

Copy link
Collaborator

@gabezurita gabezurita left a comment

Choose a reason for hiding this comment

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

Great work on laying a solid foundation here! The code LGTM overall (Python not being my main jam), and I’ve left a few questions to clarify design choices and better understand your approach! Thank you for figuring this out so fast!

vro-streamlit/src/vro_streamlit/auth/auth_service.py Outdated Show resolved Hide resolved
vro-streamlit/src/vro_streamlit/auth/auth_service.py Outdated Show resolved Hide resolved
vro-streamlit/src/vro_streamlit/auth/auth_service.py Outdated Show resolved Hide resolved
vro-streamlit/src/vro_streamlit/config.py Outdated Show resolved Hide resolved
vro-streamlit/src/vro_streamlit/auth/auth_service.py Outdated Show resolved Hide resolved
vro-streamlit/src/vro_streamlit/auth/auth_service.py Outdated Show resolved Hide resolved
vro-streamlit/src/vro_streamlit/auth/auth_frontend.py Outdated Show resolved Hide resolved
vro-streamlit/src/dev-requirements.txt Outdated Show resolved Hide resolved
Copy link
Collaborator

@gabezurita gabezurita left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@dfitchett dfitchett merged commit df2da67 into develop Oct 30, 2024
1 check passed
@dfitchett dfitchett deleted the dfitchett/streamlit-user branch October 30, 2024 21:35
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.

2 participants