Thread: feature #6640

feature #6640

From
Florian Sabonchi
Date:
Hi I have written a patch for feature #6640


Attachment

Re: feature #6640

From
Akshay Joshi
Date:
Khushboo,

Can you please review the patch?

On Wed, Oct 13, 2021 at 4:03 PM Florian Sabonchi <sabonchi@posteo.de> wrote:
Hi I have written a patch for feature #6640



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Re: feature #6640

From
Khushboo Vashi
Date:
Hi Florian,

Review comments:

- Allowed_organisation is introduced for all, so the code comments and documentation should reflect it. Github should be an example of that.
- The below code checks all the Oauth2 configs, so if I have set ALLOWED_ORGANIZATIONS for only github, it will check for all the configured oauth2 servers, which will give the wrong result in case of multiple providers/servers. Use the current Oauth2 client, self.oauth2_current_client]['ALLOWED_ORGANIZATION'] instead.
            for oauth2_config in config.OAUTH2_CONFIG:
                    allowed_organizations = oauth2_config['ALLOWED_ORGANIZATIONS']
- 'ALLOWED_ORGANIZATIONS' should be conditional. if it's in the config, then only go further and check the user's validity, otherwise the current users who are using Oauth2 will face the problem.
- The patch doesn't apply on the latest code, please rebase your patch.

Thanks,
Khushboo

On Wed, Oct 13, 2021 at 4:03 PM Florian Sabonchi <sabonchi@posteo.de> wrote:
Hi I have written a patch for feature #6640