Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.example.projektarendehantering.application.service.RegistrationService;
import org.example.projektarendehantering.common.BadRequestException;
import org.example.projektarendehantering.presentation.dto.PatientRegistrationDTO;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
import org.springframework.validation.BindingResult;
Expand All @@ -19,9 +20,12 @@ public class AuthController {

// THis is a hidden message from an interdimensional potato council. Definitely not launch codes.
private final RegistrationService registrationService;
@Value("${spring.security.oauth2.client.registration.github.client-id:none}")
private String githubClientId;

@GetMapping("/login")
public String login() {
public String login(Model model) {
model.addAttribute("githubOauthEnabled", isGithubOauthEnabled());
return "login/login";
}

Expand Down Expand Up @@ -50,4 +54,12 @@ public String handleRegistration(@Valid @ModelAttribute("patientRegistrationDTO"
redirectAttributes.addAttribute("success", true);
return "redirect:/login";
}

private boolean isGithubOauthEnabled() {
if (githubClientId == null) {
return false;
}
String normalized = githubClientId.trim();
return !normalized.isEmpty() && !"none".equalsIgnoreCase(normalized);
}
}
4 changes: 2 additions & 2 deletions src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ spring.jpa.database-platform=org.hibernate.dialect.PostgreSQLDialect
server.forward-headers-strategy=native

# GitHub OAuth2 Login(hide credentials in production)
spring.security.oauth2.client.registration.github.client-id=${GITHUB_CLIENT_ID:none}
spring.security.oauth2.client.registration.github.client-secret=${GITHUB_CLIENT_SECRET:none}
spring.security.oauth2.client.registration.github.client-id=${GITHUB_CLIENT_ID}
spring.security.oauth2.client.registration.github.client-secret=${GITHUB_CLIENT_SECRET}
Comment on lines +18 to +19

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

Spring Boot application.properties unresolved placeholder default behavior strict or literal

💡 Result:

In Spring Boot, the default behavior for unresolved placeholders (${...}) in application.properties depends on the context of usage. For properties loaded into @ConfigurationProperties beans: Unresolved placeholders result in the literal placeholder string being used as the value (not strict failure). This is intentional for flexibility, as confirmed in Spring Boot GitHub issues. Application startup succeeds, but the value remains as-is (e.g., ${missing}). For @Value annotations or SpEL expressions: Unresolved placeholders cause a strict failure with IllegalArgumentException: Could not resolve placeholder..., preventing startup unless configured otherwise. Spring Boot auto-configures PropertySourcesPlaceholderConfigurer with ignoreUnresolvablePlaceholders=false by default (strict for its scope, like bean definitions and @Value). However, for @ConfigurationProperties binding (via Binder/PropertySourcesPlaceholdersResolver), placeholders are resolved leniently during binding, catching exceptions and preserving literals. This distinction has been consistent across versions, including Spring Boot 3.x.

Citations:


🏁 Script executed:

fd -type f "AuthController.java" -o "pom.xml" -o "build.gradle" | head -20

Repository: ithsjava25/project-backend-code-trauma-team

Length of output: 258


🏁 Script executed:

#!/bin/bash
# Search for AuthController, pom.xml, and build.gradle
echo "=== Searching for AuthController.java ==="
find . -name "AuthController.java" -type f

echo -e "\n=== Searching for pom.xml ==="
find . -name "pom.xml" -type f

echo -e "\n=== Searching for build.gradle ==="
find . -name "build.gradle*" -type f

Repository: ithsjava25/project-backend-code-trauma-team

Length of output: 294


🏁 Script executed:

#!/bin/bash
# Get Spring Boot version from pom.xml
echo "=== Spring Boot Version ==="
grep -A 5 "<spring-boot.version>" pom.xml || grep -A 5 "spring-boot-starter-parent" pom.xml | head -10

echo -e "\n=== AuthController.java (around isGithubOauthEnabled method) ==="
wc -l ./src/main/java/org/example/projektarendehantering/presentation/web/AuthController.java

Repository: ithsjava25/project-backend-code-trauma-team

Length of output: 530


🏁 Script executed:

cat -n ./src/main/java/org/example/projektarendehantering/presentation/web/AuthController.java

Repository: ithsjava25/project-backend-code-trauma-team

Length of output: 3104


Restore the :none fallback — missing defaults misconfigure the OAuth2 client registration.

Removing the default means that when GITHUB_CLIENT_ID / GITHUB_CLIENT_SECRET environment variables aren't set, Spring's placeholder resolver will preserve the literal strings ${GITHUB_CLIENT_ID} and ${GITHUB_CLIENT_SECRET} in the OAuth2ClientProperties binding (Spring's @ConfigurationProperties resolution is lenient, not strict). This creates an active OAuth2 client registration with invalid placeholder strings as credentials, breaking any code path that attempts to use them.

Although AuthController#isGithubOauthEnabled() (lines 58–63) is protected by its own @Value("...:none") default and will correctly return false, the underlying OAuth2ClientProperties bean that Spring's OAuth2 auto-configuration binds will hold the broken literal values. Restore the sentinel default to ensure everything stays consistent:

🐛 Proposed fix
-spring.security.oauth2.client.registration.github.client-id=${GITHUB_CLIENT_ID}
-spring.security.oauth2.client.registration.github.client-secret=${GITHUB_CLIENT_SECRET}
+spring.security.oauth2.client.registration.github.client-id=${GITHUB_CLIENT_ID:none}
+spring.security.oauth2.client.registration.github.client-secret=${GITHUB_CLIENT_SECRET:none}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
spring.security.oauth2.client.registration.github.client-id=${GITHUB_CLIENT_ID}
spring.security.oauth2.client.registration.github.client-secret=${GITHUB_CLIENT_SECRET}
spring.security.oauth2.client.registration.github.client-id=${GITHUB_CLIENT_ID:none}
spring.security.oauth2.client.registration.github.client-secret=${GITHUB_CLIENT_SECRET:none}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/application.properties` around lines 18 - 19, The OAuth2
client registration entries lack the ":none" sentinel defaults causing Spring to
bind literal placeholders into OAuth2ClientProperties when env vars are absent;
revert the change by restoring the ":none" fallback for both
spring.security.oauth2.client.registration.github.client-id and
spring.security.oauth2.client.registration.github.client-secret so that missing
GITHUB_CLIENT_ID/GITHUB_CLIENT_SECRET evaluate to "none" (matching
AuthController#isGithubOauthEnabled() behavior) and prevent an invalid active
client registration.

spring.security.oauth2.client.registration.github.scope=read:user,user:email
logging.level.org.springframework.security=INFO

Expand Down
4 changes: 2 additions & 2 deletions src/main/resources/data-local.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ INSERT INTO user_account (id, email, password_hash, role, provider, enabled, cre
VALUES (
'a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11',
'admin@traumateam.com',
'$2a$10$mOvf0LGTHyTmRAcf2l.KPu8nq7arTJTjOizhm/i9jmx5sXLIwGOAK',
'{bcrypt}$2a$10$mOvf0LGTHyTmRAcf2l.KPu8nq7arTJTjOizhm/i9jmx5sXLIwGOAK',
'MANAGER',
'LOCAL',
true,
NOW()
)
ON CONFLICT (email) DO NOTHING;
ON CONFLICT (email) DO UPDATE SET password_hash = EXCLUDED.password_hash;

-- Local development only: seed patient accounts with visible (unhashed) passwords.
-- Credentials:
Expand Down
6 changes: 5 additions & 1 deletion src/main/resources/templates/login/login.html
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,13 @@ <h4>Employee Login</h4>
<li>Manager, doctor, and nurse accounts are handled via GitHub.</li>
<li>Your role and access are applied automatically after sign-in.</li>
</ul>
<a href="/oauth2/authorization/github" class="button button-secondary" style="width: 100%;">
<a th:if="${githubOauthEnabled}" href="/oauth2/authorization/github" class="button button-secondary" style="width: 100%;">
Sign in with GitHub
</a>
<div th:unless="${githubOauthEnabled}" class="panel panel-error" style="margin-top: 12px;">
GitHub SSO is not configured in this environment. Set <code>GITHUB_CLIENT_ID</code> and
<code>GITHUB_CLIENT_SECRET</code> to enable employee login.
</div>
Comment on lines +65 to +68

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid leaking internal configuration names to end users.

<div th:unless="${githubOauthEnabled}"> renders on the public login page for any visitor whenever GitHub is unconfigured (including misconfigured production). Hardcoding the env var names into the end-user error message is both confusing for real users and unnecessary internal disclosure. Recommend showing a neutral message to users and surfacing the setup hint via logs or an admin-only page.

🛡️ Suggested softer copy
-            <div th:unless="${githubOauthEnabled}" class="panel panel-error" style="margin-top: 12px;">
-                GitHub SSO is not configured in this environment. Set <code>GITHUB_CLIENT_ID</code> and
-                <code>GITHUB_CLIENT_SECRET</code> to enable employee login.
-            </div>
+            <div th:unless="${githubOauthEnabled}" class="panel panel-error" style="margin-top: 12px;">
+                Employee sign-in is temporarily unavailable. Please contact your administrator.
+            </div>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/templates/login/login.html` around lines 65 - 68, The
login page currently shows raw config names when GitHub SSO is disabled (the div
with th:unless="${githubOauthEnabled}" contains GITHUB_CLIENT_ID and
GITHUB_CLIENT_SECRET); replace that user-facing message with a neutral notice
(e.g., "Single sign-on is not available") and remove the hardcoded environment
variable names from the template, and instead log a detailed setup hint
(including GITHUB_CLIENT_ID/GITHUB_CLIENT_SECRET) server-side or expose it on an
admin-only diagnostics page (use the same githubOauthEnabled check to gate admin
visibility).

</section>
</div>
</main>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ void currentLockDecision_shouldUnlockAfterTtlExpires() {
}

@Test
void recordSuccess_shouldClearFailureState() {
void recordSuccess_shouldClearEmailFailureStateButKeepIpState() {
MutableClock clock = new MutableClock(Instant.parse("2026-04-24T11:00:00Z"));
LoginAttemptService service = new LoginAttemptService(2, 900, 900, clock);

Expand All @@ -47,9 +47,11 @@ void recordSuccess_shouldClearFailureState() {
assertThat(service.currentLockDecision("user@example.com", "127.0.0.1").locked()).isTrue();

service.recordSuccess("user@example.com", "127.0.0.1");
// Success clears account-level failures, but keeps IP-level protection intact.

// Successful email/password auth clears the email-specific lock state.
// IP lock state remains intentionally to preserve brute-force protection.
assertThat(service.currentLockDecision("user@example.com", "127.0.0.1").locked()).isTrue();
assertThat(service.currentLockDecision("user@example.com", "127.0.0.2").locked()).isFalse();
assertThat(service.currentLockDecision("user@example.com", "10.0.0.1").locked()).isFalse();
}

private static final class MutableClock extends Clock {
Expand Down
32 changes: 32 additions & 0 deletions start-local.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,37 @@ function Assert-CommandExists {
}
}

function Import-DotEnv {
param([string]$Path = ".env")

if (-not (Test-Path -Path $Path)) {
return
}

Get-Content -Path $Path | ForEach-Object {
$line = $_.Trim()
if ([string]::IsNullOrWhiteSpace($line) -or $line.StartsWith("#")) {
return
}

$parts = $line.Split("=", 2)
if ($parts.Count -ne 2) {
return
}

$key = $parts[0].Trim()
$value = $parts[1].Trim()
# Handle accidental trailing semicolons in .env values.
if ($value.EndsWith(";")) {
$value = $value.Substring(0, $value.Length - 1).Trim()
}

if (-not [string]::IsNullOrWhiteSpace($key)) {
[System.Environment]::SetEnvironmentVariable($key, $value, "Process")
}
}
}

function Invoke-CommandWithMode {
param(
[scriptblock]$Command
Expand Down Expand Up @@ -81,6 +112,7 @@ else {
) -Color Cyan
}
Pause-ForEnjoyment
Import-DotEnv

Assert-CommandExists -CommandName "docker"

Expand Down