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 wrote:
> Hi I have written a patch for feature #6640
>
>
