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

#1146: Implement remember me functionality using spring security. #1197

Closed
wants to merge 3 commits into from

Conversation

SteKoe
Copy link
Contributor

@SteKoe SteKoe commented Jul 2, 2019

I have implemented the requested feature "remember me #1146".

Remember me checkbox

@SteKoe SteKoe requested a review from joshiste July 2, 2019 10:18
@coveralls
Copy link

coveralls commented Jul 2, 2019

Coverage Status

Coverage decreased (-0.02%) to 91.192% when pulling ccd0d26 on feat/1146 into c0e1514 on master.

@joshiste
Copy link
Collaborator

joshiste commented Jul 3, 2019

We should either autodetect the remember me support or either provide an option to configure it so that the box is hidden when not active.

@@ -31,10 +35,21 @@
public class SecuritySecureConfig extends WebSecurityConfigurerAdapter {
private final AdminServerProperties adminServer;

@Value("${spring.security.remember-me.timeout:1209600}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this really a well known / standard property for configuring this? I can't find any docs on this. If not I'd suggest to remove this configuration option...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed that, since it is not a standard property.

@Value("${spring.security.remember-me.timeout:1209600}")
private int REMEMBER_ME_TIMEOUT;

@Value("${spring.security.remember-me.token:#{rememberMeTokenGenerator}}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this really a well known / standard property for configuring this? I can't find any docs on this. If not I'd suggest to remove this configuration option...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed that, since it is not a standard property.

@SteKoe
Copy link
Contributor Author

SteKoe commented Jul 3, 2019

I have added a config to uiSettings that allows to enable or disable remember me checkbox.

@joshiste
Copy link
Collaborator

joshiste commented Jul 3, 2019

please have a look at the checkstyle violations

@joshiste
Copy link
Collaborator

joshiste commented Jul 9, 2019

Merged with 76a6b1a and polished with 6ecee1b

@joshiste joshiste closed this Jul 9, 2019
@joshiste joshiste deleted the feat/1146 branch July 14, 2019 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants