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

Add third-party login #10

Merged
merged 55 commits into from Dec 29, 2023
Merged

Add third-party login #10

merged 55 commits into from Dec 29, 2023

Conversation

EkkoG
Copy link
Contributor

@EkkoG EkkoG commented Dec 9, 2023

For now, get GitHub user info works fine, I need to imp user table next.

  • Add user table, it saves the third-party platform info
  • Allow both local user and third-party user access configs API
  • Allow guest user access login pages without auth
  • Modulize third-party login
  • Try to make third-party login as an optional feature

Platform support

  • GitHub
  • Google
  • GitLab
  • Microsoft

Related issue #9

@EkkoG EkkoG changed the title Add github login Add github login #9 Dec 9, 2023
@EkkoG EkkoG changed the title Add github login #9 Add github login Dec 9, 2023
@EkkoG EkkoG changed the title Add github login Add GitHub login Dec 9, 2023
dependabot bot and others added 4 commits December 15, 2023 03:56
Bumps [zerocopy](https://github.com/google/zerocopy) from 0.7.29 to 0.7.31.
- [Release notes](https://github.com/google/zerocopy/releases)
- [Changelog](https://github.com/google/zerocopy/blob/main/CHANGELOG.md)
- [Commits](google/zerocopy@v0.7.29...v0.7.31)

---
updated-dependencies:
- dependency-name: zerocopy
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…opy-0.7.31

build(deps): bump zerocopy from 0.7.29 to 0.7.31
Bumps [unsafe-libyaml](https://github.com/dtolnay/unsafe-libyaml) from 0.2.9 to 0.2.10.
- [Release notes](https://github.com/dtolnay/unsafe-libyaml/releases)
- [Commits](dtolnay/unsafe-libyaml@0.2.9...0.2.10)

---
updated-dependencies:
- dependency-name: unsafe-libyaml
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…e-libyaml-0.2.10

build(deps): bump unsafe-libyaml from 0.2.9 to 0.2.10
@EkkoG EkkoG marked this pull request as ready for review December 22, 2023 09:00
@EkkoG
Copy link
Contributor Author

EkkoG commented Dec 22, 2023

For now, token will print at the page of GitHub callback, it's better to redirect to another page without the code given by GitHub, because if not, browser will create a history record with a URL contains the code, but I don't know how to pass the token to another page safety, @Clem-Fern do you have any idea?

@EkkoG
Copy link
Contributor Author

EkkoG commented Dec 23, 2023

I have saved the token to cookie and redirect to the main login page now.

@EkkoG
Copy link
Contributor Author

EkkoG commented Dec 27, 2023

Now all tasks have finished.

I had made a big change of the Dockerfile and docker-compose files, now each of them only has one file, features can be enabled by docker build-args, if you feel this change is not very good, I can remove relate commits from this PR.

For why merge two of the docker-compose.yml file, because we can start by use service name, like

docker-compose up rtabby-sqlite

@EkkoG
Copy link
Contributor Author

EkkoG commented Dec 27, 2023

One thing needs to discuss is, do you think it need to support bind an account from another platform to current user? If it's need, now it's better to change the user table, and add a third-party user table to store the platform info, one user can link to multiple user info third-party user table.

@Clem-Fern
Copy link
Owner

Clem-Fern commented Dec 27, 2023

Hey,

Again, thank's a lot for your great work.

About docker compose, I've always separate service and image in different files before. It was my way to work but I honestly never wondered if it was the right way to do. Do you think it is more user friendly with only one docker compose file ?

I will test all this in the next few days. In the same time, I'll try to write some docs about it.

@EkkoG
Copy link
Contributor Author

EkkoG commented Dec 28, 2023

I don't think there is an absolutely correct way to use it, for only one file it's easy to maintain and document the feature.

I moved the login code to a separate module, just few lines code add with feature flag at auth.rs and routes/user.rs because these codes depend on the API call in main app.

@EkkoG
Copy link
Contributor Author

EkkoG commented Dec 29, 2023

I think it not necessary to support bind multiple third-party account to one token now, even if it need to support in the future, we can add a migration, move the platform info to a separate table, then allow user to bind another account.

@EkkoG
Copy link
Contributor Author

EkkoG commented Dec 29, 2023

All the code has updated on my own server. https://tabby-sync.imciel.com/

@Clem-Fern Clem-Fern changed the base branch from master to dev December 29, 2023 21:00
@Clem-Fern Clem-Fern merged commit 4465e9f into Clem-Fern:dev Dec 29, 2023
6 checks passed
@Clem-Fern
Copy link
Owner

Writing a bit of docs about this feature and then merging dev into master to release a new version.

Thank's again ;)

Clem-Fern added a commit that referenced this pull request Feb 1, 2024
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.

None yet

2 participants