Skip to content

Feature/#198 password encoding #199

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.

Sign up for GitHub

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

Open
mplieske wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Feature/#198 password encoding #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,12 @@
<version>2.18.2</version>
</dependency>

<!-- for Argon2PasswordEncoder -->
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk18on</artifactId>

@Death111 Death111 Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I guess I would rather use a spring buildin passwordencoder like https://docs.spring.io/spring-security/reference/features/authentication/password-storage.html (I did not validate this yet.) But I like less dependencies :P

@mplieske mplieske Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The Argon2PasswordEncoder is mentioned on the page you linked :)
https://docs.spring.io/spring-security/reference/features/authentication/password-storage.html#authentication-password-storage-argon2

To be fair, we could use the default password encoder. Probably it will be sufficient for KeepTime 😄

@Death111 Death111 Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you are right. argon is mentioned

the default one sound also sufficient - saves 8MB of disk space :D

<version>1.78.1</version> <!-- or latest -->
</dependency>
</dependencies>
<build>
<finalName>keeptime-${project.version}</finalName>
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/de/doubleslash/keeptime/common/DefaultPasswordEncoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package de.doubleslash.keeptime.common;

import org.springframework.security.crypto.argon2.Argon2PasswordEncoder;

public class DefaultPasswordEncoder {

private static Argon2PasswordEncoder passwordEncoder = new Argon2PasswordEncoder(16, 32, 4, 128000, 10);

public static final Argon2PasswordEncoder getPasswordEncoder() {
return DefaultPasswordEncoder.passwordEncoder;
}

}
8 changes: 8 additions & 0 deletions src/main/java/de/doubleslash/keeptime/rest/SecurityConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
import org.springframework.security.config.http.SessionCreationPolicy;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.web.SecurityFilterChain;

import de.doubleslash.keeptime.common.DefaultPasswordEncoder;

@Configuration
@EnableWebSecurity
public class SecurityConfiguration {
Expand All @@ -39,4 +42,9 @@ public SecurityFilterChain filterChain(final HttpSecurity http) throws Exception

return http.build();
}

@Bean
public PasswordEncoder passwordEncoder() {
return DefaultPasswordEncoder.getPasswordEncoder();
}
}
6 changes: 5 additions & 1 deletion src/main/java/de/doubleslash/keeptime/view/SettingsController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.stereotype.Component;

import de.doubleslash.keeptime.ApplicationProperties;
Expand Down Expand Up @@ -758,12 +759,15 @@ private void handleApiOn() {
String username = authName.getText();
String password = authPassword.getText();

@Death111 Death111 Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

like you commented in the issue
"The configuration screen reads the password from the application.properties and loads it into the view. If you already have a configuration for the REST API feature and you want to change something, eg the port, the already hashed password will be hashed and saved."

Your idea -> "Probably we should only save a password if provided a new one and we should not load the password from the application.properties into the view"

sounds reasonable, yes 👍


PasswordEncoder passwordEncoder = DefaultPasswordEncoder.getPasswordEncoder();
String encodedPassword = passwordEncoder.encode(password);

Map<String, String> propertiesToUpdate = new HashMap<>();
propertiesToUpdate.put("spring.main.web-application-type", "");
propertiesToUpdate.put("server.port", authPort.getText());
propertiesToUpdate.put("api", "ON");
propertiesToUpdate.put("spring.security.user.name", username);
propertiesToUpdate.put("spring.security.user.password", password);
propertiesToUpdate.put("spring.security.user.password", encodedPassword);

propertyWrite(propertiesToUpdate);
}
Expand Down
Loading