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

Multi Auth #1612

Merged
merged 40 commits into from
Dec 29, 2023
Merged

Multi Auth #1612

merged 40 commits into from
Dec 29, 2023

Conversation

siddhantk232
Copy link
Contributor

@siddhantk232 siddhantk232 commented Dec 16, 2023

This PR adds emailpassword login (sign in using email and password) on top of the current github oauth. Both github oauth and emailpassword strategy create a fastn_user (database table). See multiauth.sql for more details on the required tables for authentication.

  • you're not allowed to login simulatenously with github and emailpassword.
    It'll be possible in future to merge two accounts by logging in to two
    accounts.
  • You can either use emailpassword or github. The password set in emailpassword
    step will not be considered when logged in using github
  • tested with login, logout, github login/logout, and the user-details
    processors. The user-details processor requires no change in existing
    codebases. Expect bugs in other places.

TODO

  • DONE Refactor code that makes database queries in emailpassword.rs. The mod db uses code copied from the sql processor. Ideally they should share the db pool. This will be fixed in a new PR.
  • Write tests. It is very hard without doing major refactors so it is ignored in this PR so as to quickly move to figuring out sending emails. Tests will also be added for this code in a new PR later.

remove the GhUserDetails struct
rename login to username
create fastn_user on github oauth callback
fix: user_details processors handle empty cookie string that occurs
right after logout event

fix: remove password field from create_user response
multiauth.sql Outdated Show resolved Hide resolved
multiauth.sql Outdated Show resolved Hide resolved
multiauth.sql Outdated Show resolved Hide resolved
multiauth.sql Outdated Show resolved Hide resolved
multiauth.sql Outdated Show resolved Hide resolved
multiauth.sql Outdated Show resolved Hide resolved
fastn-core/src/auth/emailpassword.rs Outdated Show resolved Hide resolved
fastn-core/src/auth/emailpassword.rs Outdated Show resolved Hide resolved
fastn-core/src/auth/emailpassword.rs Outdated Show resolved Hide resolved
fastn-core/src/auth/emailpassword.rs Outdated Show resolved Hide resolved
fastn-core/src/auth/emailpassword.rs Outdated Show resolved Hide resolved
fastn-core/src/auth/emailpassword.rs Outdated Show resolved Hide resolved
fastn-core/src/auth/mod.rs Outdated Show resolved Hide resolved
fastn-core/src/auth/mod.rs Show resolved Hide resolved
fastn-core/src/auth/routes.rs Outdated Show resolved Hide resolved
send SMTP emails in fastn
@siddhantk232
Copy link
Contributor Author

Thanks @amitu for the review. I'll fix the issues raised, add tests and, integrate diesel orm as you suggested.

.envrc Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
diesel.toml Show resolved Hide resolved
fastn-core/Cargo.toml Outdated Show resolved Hide resolved
@amitu
Copy link
Contributor

amitu commented Dec 18, 2023

@siddhantk232 also checkout https://github.com/orgs/fastn-stack/discussions/1449, and start thinking of ways we can make progress there.

and mailer instantiation errror
Cargo.toml Outdated Show resolved Hide resolved
fastn-core/src/auth/mod.rs Outdated Show resolved Hide resolved
fastn-core/src/auth/mod.rs Outdated Show resolved Hide resolved
amitu
amitu previously approved these changes Dec 27, 2023
@siddhantk232
Copy link
Contributor Author

a demo created using changes introduced in this PR: https://github.com/siddhantk232/fastn-test-multiauth

@amitu I think we can merge this PR now. Remaining things will be done in a new PR

Signed-off-by: Amit Upadhyay <upadhyay@gmail.com>
@amitu amitu disabled auto-merge December 29, 2023 05:09
@amitu amitu closed this Dec 29, 2023
@amitu amitu reopened this Dec 29, 2023
@amitu amitu merged commit 8bee550 into main Dec 29, 2023
2 checks passed
@amitu amitu deleted the feat/multiauth branch December 29, 2023 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants