From: | Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com> |
---|---|
To: | Florian Sabonchi <sabonchi(at)posteo(dot)de> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: feature #6640 |
Date: | 2021-10-18 05:07:56 |
Message-ID: | CAFOhELf_frX5vtFECJfkdzu4Sar5hGa0qrZnm4Mf2g55ttBY6g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
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(at)posteo(dot)de> wrote:
> Hi I have written a patch for feature #6640
>
>
From | Date | Subject | |
---|---|---|---|
Next Message | Khushboo Vashi | 2021-10-18 06:14:56 | Re: Patch for Bug #6832 |
Previous Message | Akshay Joshi | 2021-10-16 07:29:48 | Re: [pgAdmin][RM6891] Composite foreign keys in ERD |