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

Col-1073 Update GitHub auth #79

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Conversation

matthuynh
Copy link
Member

@matthuynh matthuynh commented May 9, 2020

Changelog:

  • Moved all OAuth variables (client ID and client secret) into .env files for both client and server
    • For client, make a .env file in client/.env, and add this line to it:
      REACT_APP_GITHUB_CLIENT_ID=<client-id-goes-here>
  • Change path of localhost:3000/register2 to now lead to localhost:3000/register/authenticated. This is the URL that GitHub redirects to after the user authorizes the application
  • Styling changes in Login and Register pages. Also added tooltips (eg. hovering over "email" field explains to user why we need their email)
  • Minor refactoring

Testing:

  • Paste the values of Client ID and Client Secret from here into you server/config/.env
  • Paste JUST Client ID into client/.env
  • Start the application, and see if you can still register

Extra Note:
Adding an .env file in the client still does not prevent users from digging out the client ID if they REALLY want to. From some quick Googling, it seems like the worst an attacker could do with our client ID is for phishing.

Closes #46
Closes #73

@matthuynh matthuynh added enhancement New feature or request refactor Clean up existing code labels May 9, 2020
@matthuynh matthuynh self-assigned this May 9, 2020
@matthuynh matthuynh changed the title Col 1073 update GitHub auth Col 1073 Update GitHub auth May 9, 2020
@matthuynh matthuynh changed the title Col 1073 Update GitHub auth Col-1073 Update GitHub auth May 9, 2020
Copy link
Contributor

@jcserv jcserv left a comment

Choose a reason for hiding this comment

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

Just gave this a go, looks great! The changes to registration are really good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor Clean up existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants