Skip to content

Conversation

@ihsaan-ullah
Copy link
Collaborator

@ihsaan-ullah ihsaan-ullah commented Feb 1, 2024

@ mention of reviewers

@Didayolo @cjh1

A brief description of the purpose of the changes contained in this PR.

This is PR-1 for changes needed for feature: #1298

Platform owners can now enable/disable regular signup and login. This is useful for dedicated instances where people just want Organization authentication rather than using the normal sign in and signup.

  • If you disable signup i.e. set ENABLE_SIGN_UP=False in your .env, you will not see sign up button and sign up page
  • If you disable sigin i.e. set ENABLE_SIGN_IN=False in your .env, you will see empty login page. This is on purpose because you will see organization authentication buttons there (soon)

Note: if you change either of these variables in your .env you need to stop and start the service:

docker-compose down
docker-compose up -d

Issues this PR resolves

Checklist

  • Code review by me
  • Hand tested by me
  • I'm proud of my work
  • Code review by reviewer
  • Hand tested by reviewer
  • CircleCi tests are passing
  • Ready to merge

@ihsaan-ullah ihsaan-ullah mentioned this pull request Feb 1, 2024
5 tasks
@ihsaan-ullah ihsaan-ullah changed the title added env option to enable/disable regular signin and sign up Enable/Disable regular Login and Signup Feb 1, 2024
@ihsaan-ullah ihsaan-ullah changed the title Enable/Disable regular Login and Signup OIDC PR#1 --- Enable/Disable regular Login and Signup Feb 3, 2024
@bbearce
Copy link
Collaborator

bbearce commented Mar 5, 2024

Seems like it is implemented correctly. Testing all combinations we see these screenshots:

ENABLE_SIGN_UP=False ENABLE_SIGN_IN=False

/signup doesn't work
/login doesn't work
image

ENABLE_SIGN_UP=False ENABLE_SIGN_IN=True

/signup doesn't work
image

ENABLE_SIGN_UP=True ENABLE_SIGN_IN=False

image

/login doesn't work
image

ENABLE_SIGN_UP=True ENABLE_SIGN_IN=True

image
image

@bbearce
Copy link
Collaborator

bbearce commented Mar 5, 2024

Also reviewed the code and I feel that it makes sense. I see:

  • some clean up (deletion of already commented code - routes)
  • context_processor creation to allow new env variables to propagate to templates for conditional rendering

@Didayolo Didayolo merged commit 641a247 into develop Mar 11, 2024
@Didayolo Didayolo deleted the oidc branch March 11, 2024 09:49
@Didayolo
Copy link
Member

@ihsaan-ullah @bbearce Is this new change documented in the Wiki?

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.

4 participants