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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make automatic login configurable #131

Merged
merged 9 commits into from
Oct 18, 2018

Conversation

dariadomagala-sap
Copy link
Contributor

Description

Changes proposed in this pull request:

  • Make automatic login configurable

Related issue(s)

Copy link
Contributor

@maxmarkus maxmarkus left a comment

Choose a reason for hiding this comment

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

Name change required: disableAutoLogin assumes it is disabled by default (since all our settings default to true. Would name it the opposite way like autoLogin.

Copy link
Contributor

@maxmarkus maxmarkus left a comment

Choose a reason for hiding this comment

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

LGTM

if (!getConfigValue('auth.disableAutoLogin')) {
this.startAuthorization();
return;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Would try to avoid the evaluation negation here, as it is not necessary.

if (getConfigValue('auth.disableAutoLogin')) {
  return;
};
this.startAuthorization();
return

Hint: if the config value would be truthy, we could do something like that:

return getConfigValue('auth.autoLogin') && this.startAuthorization();

@@ -1,13 +1,14 @@
# Authorization Configuration

Luigi provides OpenID Connect and OAuth2 Implicit Grant authorization out of the box. The **use** key defines the active authorization provider.
Luigi provides OpenID Connect and OAuth2 Implicit Grant authorization out of the box. The **use** key defines the active authorization provider and the **disableAutoLogin** key allows to disable the automatic login flow that is provided by default.
Copy link

Choose a reason for hiding this comment

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

allows to -> allows you to

@kwiatekus kwiatekus merged commit 5c001aa into SAP:master Oct 18, 2018
@kwiatekus kwiatekus deleted the autologin-configurable branch October 18, 2018 08:20
stanleychh pushed a commit to stanleychh/luigi that referenced this pull request Dec 30, 2021
* Add the possibility to disable auto-login
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants