yezz123 / authx

Ready-to-use and customizable Authentications and Oauth2 management for FastAPI ✨
https://authx.yezz.me/
MIT License
796 stars 48 forks source link

There is ambiguous loop in resolve_user for social auth #265

Closed iamromandev closed 1 year ago

iamromandev commented 2 years ago

First Check

Example Code

while True: 
     resolved_username = f"{username}{postfix}" 
     existing_username = await self._repo.get_by_username(resolved_username) 
     if not existing_username: 
         break

Description

Could you please some one check this below code base where loop is indicating something ambiguous .

https://github.com/yezz123/authx/blob/eefba52c32222b88171e4b300d025937496d7e1f/authx/services/social.py#L127-L131

Operating System

macOS

Operating System Details

No response

FastAPI Version

0.85.0

Python Version

3.9

Additional Context

No response

iamromandev commented 2 years ago

I believe some thing like below could solve this loop issue.

            resolved_username = username
            while True:
                postfix = str(i) if i > 0 else ""
                resolved_username = f"{username}{postfix}"
                existing_username = await self._repo.get_by_username(resolved_username)
                if not existing_username:
                    break
                i = i+1
yezz123 commented 2 years ago

Hello, @iftenet I didn't notice that loop, but I see it can be fixed as you proposed, want to contribute and open a Pull request?

iamromandev commented 2 years ago

Hello, @iftenet I didn't notice that loop, but I see it can be fixed as you proposed, want to contribute and open a Pull request?

I will be glad if you give me the opportunity to contribute.

yezz123 commented 2 years ago

Hello, @iftenet I didn't notice that loop, but I see it can be fixed as you proposed, want to contribute and open a Pull request?

I will be glad if you give me the opportunity to contribute.

Yes for sure!

I will assign it as a bug and you can work on it 🚀

iamromandev commented 1 year ago

@yezz123 I am going to create a PR but seems I have no permission to push.

ERROR: Permission to yezz123/authx.git denied to iftenet.

yezz123 commented 1 year ago

@yezz123 I am going to create a PR but seems I have no permission to push.

ERROR: Permission to yezz123/authx.git denied to iftenet.

You can fork the project and try to made your changes in your fork you will find it in this way, iftenet/authx.git then a yellow head with a button create PR