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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add discord and bitbucket provider #159

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

afi-dev
Copy link

@afi-dev afi-dev commented Apr 30, 2024

Description

This pull request adds support for 馃棟Discord and 馃棟Bitbucket single sign-on (SSO) providers to fastapi-sso. These additions extend authentication options. I added it because I personally needed it 馃槉, and I think it will be useful to others.

Changes Made

  • Discord SSO Provider: A new integration has been added to allow users to sign by using their Discord accounts. See fastapi_sso/sso/discord.py

discordsso

  • Atlassian Bitbucket SSO Provider: A new integration has been added to allow users to sign by using their BitBucket accounts. See fastapi_sso/sso/bitbucket.py

bitbucketsso

Tests Performed

  • Pytest 馃憣
  • Personal integration test 馃憣

Typechecking

  • black 馃憣
  • pylint 馃憣
  • mypy 馃憣
  • pyclean 馃憣
  • pre-commit 馃憣

Exemples

  • MKDocs has been updated 馃憤

Examples of use have been added :

docs/generate_reference.py Fixed Show fixed Hide fixed
Copy link
Owner

@tomasvotava tomasvotava left a comment

Choose a reason for hiding this comment

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

Thanks a lot, whant an awesome contribution! Could you please take a look at those two minor nitpicks I found? Also the pipeline is failing, could you take a look at that? I don't understand why the similarities check does not ignore function annotations, feel free to add ignore comment. If you need help with anything, let me know. Nice work!

docs/generate_reference.py Outdated Show resolved Hide resolved
fastapi_sso/sso/discord.py Outdated Show resolved Hide resolved
@afi-dev
Copy link
Author

afi-dev commented May 2, 2024

I think I've applied the changes correctly, but please let me know if you need any further changes.

Copy link
Owner

@tomasvotava tomasvotava left a comment

Choose a reason for hiding this comment

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

Please take a look at the last thing, also, the pipeline is still failing, I am more of a ruff fan lately, so ignore it for now, I'll deal with it.

fastapi_sso/sso/discord.py Outdated Show resolved Hide resolved
Apply fix

Co-authored-by: Tomas Votava <[email protected]>
@tomasvotava
Copy link
Owner

It looks perfect now, could you please try rebasing from the master? I switched to ruff, I hope it won't break anything, but the pipeline should now pass.

git checkout master
git pull
git checkout -
git rebase master

That should do it. There will probably be some conflicts, I can help you resolve them if you have any troubles.

@tomasvotava
Copy link
Owner

I felt bad for requesting so many changes, so I went ahead and created a PR into your PR afi-dev#2 馃榿 it's all rebased, you only have to accept all incoming to resolve the conflicts. If you'd like to resolve yourself, feel free to close the pull request.

@afi-dev
Copy link
Author

afi-dev commented May 3, 2024

no problem

fastapi_sso/__init__.py Outdated Show resolved Hide resolved
Fix minor misscheck
Copy link
Author

@afi-dev afi-dev left a comment

Choose a reason for hiding this comment

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

For me it's fine, I hope it's the last fix and that I haven't forgotten anything, after checking, there's nothing missing.

@afi-dev afi-dev requested a review from tomasvotava May 10, 2024 22:27
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