feat: improve security and prevent xss attacks (#30)

* feat: improve security and prevent xss attacks

* chore: fix frontend test file name location

* fix: frontend test case

* ci: add deployment connection

.github/workflows/cd-pr.yml

@@ -9,6 +9,7 @@ on:
 jobs:
   terraform-plan-stg:
     name: Plan changs to staging
+    if: ${{ github.event.workflow_run.conclusion == 'success' }}
     runs-on: ubuntu-latest
     outputs:
       no_changes: ${{ steps.check-changes.outputs.no_changes }}

.github/workflows/ci-pr-backend.yml

@@ -51,6 +51,7 @@ jobs:
     needs: ["run-checks"]
     permissions:
       contents: read
+      deployments: write
       packages: write
 
     steps:
@@ -100,3 +101,29 @@ jobs:
           docker tag ghcr.io/${{ steps.repo.outputs.name }}/api:latest ghcr.io/${{ steps.repo.outputs.name }}/api:pr-${{ github.event.pull_request.number }}
           docker push ghcr.io/${{ steps.repo.outputs.name }}/api:candidate
           docker push ghcr.io/${{ steps.repo.outputs.name }}/api:pr-${{ github.event.pull_request.number }}
+
+      - name: Create GitHub deployment for staging
+        if: ${{ github.event_name == 'pull_request' }}
+        uses: actions/github-script@v6
+        with:
+          script: |
+            const ref = context.payload.pull_request.head.sha;
+            const env = 'staging';
+            const resp = await github.rest.repos.createDeployment({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              ref,
+              required_contexts: [],
+              environment: env,
+              description: `PR #${context.payload.pull_request.number} preview deployment`,
+              transient_environment: true,
+              auto_merge: false
+            });
+            // create a deployment status pointing to the staging URL
+            await github.rest.repos.createDeploymentStatus({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              deployment_id: resp.data.id,
+              state: 'success',
+              environment_url: 'https://tasknote-stg.darkroasted.vps-kinghost.net'
+            });

.github/workflows/ci-pr-frontend.yml

@@ -60,6 +60,7 @@ jobs:
     permissions:
       contents: write
       packages: write
+      deployments: write
 
     steps:
       - name: Checkout code
@@ -105,3 +106,29 @@ jobs:
           cache-to: type=gha,mode=max
           build-args: |
             VITE_BUILD=${{ steps.version.outputs.tag }}
+
+      - name: Create GitHub deployment for staging
+        if: ${{ github.event_name == 'pull_request' }}
+        uses: actions/github-script@v6
+        with:
+          script: |
+            const ref = context.payload.pull_request.head.sha;
+            const env = 'staging';
+            const resp = await github.rest.repos.createDeployment({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              ref,
+              required_contexts: [],
+              environment: env,
+              description: `PR #${context.payload.pull_request.number} preview deployment`,
+              transient_environment: true,
+              auto_merge: false
+            });
+            // create a deployment status pointing to the staging URL
+            await github.rest.repos.createDeploymentStatus({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              deployment_id: resp.data.id,
+              state: 'success',
+              environment_url: 'https://tasknote-stg.darkroasted.vps-kinghost.net'
+            });

client/src/__test__/utils/UrlUtils.test.ts

@@ -0,0 +1,35 @@
+import { describe, it, expect } from 'vitest';
+import { isSafeUrl } from '../../utils/UrlUtils';
+
+describe('UrlUtils', () => {
+  it('should allow http:// URLs', () => {
+    expect(isSafeUrl('http://example.com')).toBe(true);
+  });
+
+  it('should allow https:// URLs', () => {
+    expect(isSafeUrl('https://example.com')).toBe(true);
+  });
+
+  it('should allow # URLs', () => {
+    expect(isSafeUrl('#section')).toBe(true);
+  });
+
+  it('should disallow javascript: URLs', () => {
+    expect(isSafeUrl('javascript:alert(1)')).toBe(false);
+  });
+
+  it('should disallow data: URLs', () => {
+    expect(isSafeUrl('data:text/html,<script>alert(1)</script>')).toBe(false);
+  });
+
+  it('should disallow empty or null URLs', () => {
+    expect(isSafeUrl('')).toBe(false);
+    expect(isSafeUrl(null)).toBe(false);
+    expect(isSafeUrl(undefined)).toBe(false);
+  });
+
+  it('should be case insensitive for protocol', () => {
+    expect(isSafeUrl('HTTP://example.com')).toBe(true);
+    expect(isSafeUrl('HTTPS://example.com')).toBe(true);
+  });
+});

client/src/components/NoteTitle/index.tsx

@@ -1,5 +1,6 @@
 import React from 'react';
 import ExternalLinkIcon from '../../assets/icons8-external-link-30.png';
+import { isSafeUrl } from '../../utils/UrlUtils';
 
 interface Props {
   readonly title: string;
@@ -18,8 +19,8 @@ function NoteTitle(props: React.PropsWithChildren<Props>): React.ReactNode {
     <span className="task-title-icon">
       <span className="poppins-semibold">
         {props.title}
-        {props.noteUrl && props.noteUrl.length > 0 && (
-          <a href={props.noteUrl} target="_blank" rel="noreferrer" className="task-note-external-link">
+        {isSafeUrl(props.noteUrl) && (
+          <a href={props.noteUrl!} target="_blank" rel="noreferrer" className="task-note-external-link">
             <img src={ExternalLinkIcon} width={20} alt="external link" />
           </a>
         )}

client/src/components/TaskTitle/index.tsx

@@ -1,5 +1,6 @@
 import React from 'react';
 import ExternalLinkIcon from '../../assets/icons8-external-link-30.png';
+import { isSafeUrl } from '../../utils/UrlUtils';
 import './style.css';
 
 interface Props {
@@ -24,7 +25,7 @@ function TaskTitle(props: React.PropsWithChildren<Props>): React.ReactNode {
         data-testid={`task-title-text-${props.title}`}
       >
         {props.title}
-        {props.taskUrl && props.taskUrl.length > 0 && (
+        {props.taskUrl && props.taskUrl.length > 0 && isSafeUrl(props.taskUrl[0]) && (
           <a href={props.taskUrl[0]} target="_blank" rel="noreferrer" className="task-note-external-link">
             <img src={ExternalLinkIcon} width={20} alt="external link" />
           </a>

client/src/utils/UrlUtils.ts

@@ -0,0 +1,14 @@
+/**
+ * Validates if a URL is safe to be used in an <a> tag.
+ * Only allows http, https, and # (for internal links/placeholders).
+ *
+ * @param {string | null | undefined} url The URL to validate.
+ * @returns {boolean} True if the URL is safe, false otherwise.
+ */
+export function isSafeUrl(url: string | null | undefined): boolean {
+  if (!url) {
+    return false;
+  }
+  const safeProtocolRegex = /^(https?:\/\/|#)/i;
+  return safeProtocolRegex.test(url);
+}

client/src/views/SharedNote/index.tsx

@@ -6,6 +6,7 @@ import remarkGfm from 'remark-gfm';
 import { NoteResponse } from '../../types/NoteResponse';
 import api from '../../api-service/api';
 import ApiConfig from '../../api-service/apiConfig';
+import { isSafeUrl } from '../../utils/UrlUtils';
 
 /**
  * SharedNote component for displaying a publicly shared note.
@@ -80,9 +81,9 @@ function SharedNote(): React.ReactNode {
             </Card.Header>
             <Card.Body>
               <Card.Title>{note.title}</Card.Title>
-              {note.url && (
+              {isSafeUrl(note.url) && (
                 <p>
-                  <a href={note.url} target="_blank" rel="noopener noreferrer">
+                  <a href={note.url!} target="_blank" rel="noopener noreferrer">
                     {note.url}
                   </a>
                 </p>

server/src/main/java/br/com/tasknoteapp/server/entity/UserEntity.java

@@ -55,6 +55,9 @@ public class UserEntity implements UserDetails {
   @Column(name = "lang", nullable = true, length = 6)
   private String lang;
 
+  @Column(name = "last_password_change", nullable = false)
+  private LocalDateTime lastPasswordChange;
+
   @Override
   public Collection<? extends GrantedAuthority> getAuthorities() {
     return List.of();
@@ -182,4 +185,12 @@ public class UserEntity implements UserDetails {
   public void setLang(String lang) {
     this.lang = lang;
   }
+
+  public LocalDateTime getLastPasswordChange() {
+    return lastPasswordChange;
+  }
+
+  public void setLastPasswordChange(LocalDateTime lastPasswordChange) {
+    this.lastPasswordChange = lastPasswordChange;
+  }
 }

server/src/main/java/br/com/tasknoteapp/server/request/NotePatchRequest.java

@@ -1,4 +1,13 @@
 package br.com.tasknoteapp.server.request;
 
+import jakarta.validation.constraints.Pattern;
+
 /** This record represents a note patch payload. */
-public record NotePatchRequest(String title, String description, String url, String tag) {}
+public record NotePatchRequest(
+    String title,
+    String description,
+    @Pattern(
+            regexp = "^(https?://.*|#.*)?$",
+            message = "URL must start with http://, https:// or #")
+        String url,
+    String tag) {}

server/src/main/java/br/com/tasknoteapp/server/request/NoteRequest.java

@@ -1,7 +1,14 @@
 package br.com.tasknoteapp.server.request;
 
 import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Pattern;
 
 /** This record represents a note request to be created. */
 public record NoteRequest(
-    @NotNull String title, @NotNull String description, String url, String tag) {}
+    @NotNull String title,
+    @NotNull String description,
+    @Pattern(
+            regexp = "^(https?://.*|#.*)?$",
+            message = "URL must start with http://, https:// or #")
+        String url,
+    String tag) {}

server/src/main/java/br/com/tasknoteapp/server/request/TaskPatchRequest.java

@@ -1,12 +1,18 @@
 package br.com.tasknoteapp.server.request;
 
+import jakarta.validation.constraints.Pattern;
 import java.util.List;
 
 /** This record represents a task patch payload. */
 public record TaskPatchRequest(
     String description,
     Boolean done,
-    List<String> urls,
+    List<
+            @Pattern(
+                regexp = "^(https?://.*|#.*)?$",
+                message = "URL must start with http://, https:// or #")
+            String>
+        urls,
     String dueDate,
     Boolean highPriority,
     String tag) {}

server/src/main/java/br/com/tasknoteapp/server/request/TaskRequest.java

@@ -2,12 +2,18 @@ package br.com.tasknoteapp.server.request;
 
 import jakarta.validation.constraints.NotEmpty;
 import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Pattern;
 import java.util.List;
 
 /** This record represents a task request to be created. */
 public record TaskRequest(
     @NotNull @NotEmpty String description,
-    List<String> urls,
+    List<
+            @Pattern(
+                regexp = "^(https?://.*|#.*)?$",
+                message = "URL must start with http://, https:// or #")
+            String>
+        urls,
     String dueDate,
     Boolean highPriority,
     String tag) {}

server/src/main/java/br/com/tasknoteapp/server/service/AuthService.java

@@ -27,6 +27,7 @@ import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.time.Duration;
 import java.time.LocalDateTime;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -131,7 +132,8 @@ public class AuthService {
     user.setEmail(newUser.email());
     user.setPassword(passwordEncoder.encode(newUser.password()));
     user.setAdmin(false);
-    user.setCreatedAt(LocalDateTime.now());
+    user.setCreatedAt(LocalDateTime.now().truncatedTo(ChronoUnit.SECONDS));
+    user.setLastPasswordChange(LocalDateTime.now().truncatedTo(ChronoUnit.SECONDS));
     user.setEmailUuid(emailUuid);
     user.setLang(newUser.lang());
     userRepository.save(user);
@@ -332,6 +334,7 @@ public class AuthService {
       }
 
       currentUser.setPassword(passwordEncoder.encode(patchRequest.password()));
+      currentUser.setLastPasswordChange(LocalDateTime.now().truncatedTo(ChronoUnit.SECONDS));
       shouldUpdate = true;
     }
 
@@ -433,7 +436,8 @@ public class AuthService {
 
     UserEntity user = userOptional.get();
     user.setResetToken(resetToken);
-    user.setResetPasswordExpiration(LocalDateTime.now().plusHours(2L));
+    user.setResetPasswordExpiration(
+        LocalDateTime.now().truncatedTo(ChronoUnit.SECONDS).plusHours(2L));
 
     userRepository.save(user);
     if (hasValidMailgunApiKey()) {
@@ -477,6 +481,7 @@ public class AuthService {
     user.setResetToken(null);
     user.setResetPasswordExpiration(null);
     user.setPassword(passwordEncoder.encode(request.password()));
+    user.setLastPasswordChange(LocalDateTime.now().truncatedTo(ChronoUnit.SECONDS));
 
     userRepository.save(user);
     if (hasValidMailgunApiKey()) {

server/src/main/java/br/com/tasknoteapp/server/service/impl/JwtServiceImpl.java

@@ -51,6 +51,14 @@ class JwtServiceImpl implements JwtService {
     return null;
   }
 
+  private LocalDateTime extractIssuedAt(String token) {
+    Date date = extractClaim(token, Claims::getIssuedAt);
+    if (!Objects.isNull(date)) {
+      return date.toInstant().atZone(java.time.ZoneId.systemDefault()).toLocalDateTime();
+    }
+    return null;
+  }
+
   @Override
   public String generateToken(UserEntity user) {
     Map<String, Object> claims = new HashMap<>();
@@ -91,7 +99,18 @@ class JwtServiceImpl implements JwtService {
   @Override
   public boolean validateTokenAndUser(String token, UserDetails user) {
     final String email = user.getUsername();
-    return !isTokenExpired(token) && email.equals(getEmailFromToken(token));
+    boolean basicValid = !isTokenExpired(token) && email.equals(getEmailFromToken(token));
+
+    if (basicValid && user instanceof UserEntity userEntity) {
+      LocalDateTime iat = extractIssuedAt(token);
+      if (iat != null && userEntity.getLastPasswordChange() != null) {
+        // Token must be issued after or at the same time as last password change
+        // We use isBefore to invalidate tokens issued BEFORE the change
+        return !iat.isBefore(userEntity.getLastPasswordChange());
+      }
+    }
+
+    return basicValid;
   }
 
   private <T> T extractClaim(String token, Function<Claims, T> claimsResolver) {

server/src/main/resources/db/migration/V202604161000__add_user_last_password_change.sql

@@ -0,0 +1,6 @@
+ALTER TABLE tasknote.users ADD COLUMN last_password_change TIMESTAMP WITHOUT TIME ZONE;
+
+-- Initialize for existing users
+UPDATE tasknote.users SET last_password_change = created_at WHERE last_password_change IS NULL;
+
+ALTER TABLE tasknote.users ALTER COLUMN last_password_change SET NOT NULL;

server/src/test/java/br/com/tasknoteapp/server/controller/TaskControllerTest.java

@@ -276,7 +276,7 @@ class TaskControllerTest {
         """
         {
           "description": "Test task",
-          "urls": ["www.url.com"],
+          "urls": ["https://www.url.com"],
           "highPriority": true,
           "tag": "tag"
         }

server/src/test/java/br/com/tasknoteapp/server/service/impl/JwtServiceImplTest.java

@@ -166,6 +166,30 @@ class JwtServiceImplTest {
     assertFalse(valid);
   }
 
+  @Test
+  void validateTokenAndUser_shouldReturnFalseIfTokenIssuedBeforeLastPasswordChange()
+      throws InterruptedException {
+    UserEntity user = new UserEntity();
+    user.setId(testUserId);
+    user.setEmail(testEmail);
+    user.setAdmin(false);
+    user.setName(testName);
+    user.setLastPasswordChange(LocalDateTime.now().truncatedTo(ChronoUnit.SECONDS));
+
+    // Token issued NOW
+    String token = jwtService.generateToken(user);
+
+    // Update lastPasswordChange to FUTURE (simulating a password change after token issuance)
+    // We wait 1 second to ensure the new timestamp is strictly after token iat (which has second
+    // precision)
+    Thread.sleep(1100);
+    user.setLastPasswordChange(LocalDateTime.now().truncatedTo(ChronoUnit.SECONDS));
+
+    boolean valid = jwtService.validateTokenAndUser(token, user);
+
+    assertFalse(valid, "Token issued before password change should be invalid");
+  }
+
   private Claims extractClaims(String token) {
     return Jwts.parser().verifyWith(getKey()).build().parseSignedClaims(token).getPayload();
   }

server/src/test/resources/sql/TaskRepositoryTest.sql

@@ -1,6 +1,6 @@
 -- Create test user
-insert into users (email, password, admin, created_at, inactivated_at)
-select 'test@domain.com', 'a1b2c3d4f5g6', false, current_timestamp, null
+insert into users (email, password, admin, created_at, inactivated_at, last_password_change)
+select 'test@domain.com', 'a1b2c3d4f5g6', false, current_timestamp, null, current_timestamp
 where not exists (select 1 from users where email = 'test@domain.com');
 
 -- Create some tasks

server/src/test/resources/sql/TaskUrlRepositoryTest.sql

@@ -1,6 +1,6 @@
 -- Create test user
-insert into users (email, password, admin, created_at, inactivated_at)
-select 'test@domain.com', 'a1b2c3d4f5g6', false, current_timestamp, null
+insert into users (email, password, admin, created_at, inactivated_at, last_password_change)
+select 'test@domain.com', 'a1b2c3d4f5g6', false, current_timestamp, null, current_timestamp
 where not exists (select 1 from users where email = 'test@domain.com');
 
 -- Create a task

server/src/test/resources/sql/UserRepositoryTest.sql

@@ -1,4 +1,4 @@
 -- Create test user
-insert into users (email, password, admin, created_at, inactivated_at, email_uuid, reset_token)
-select 'testuuid@domain.com', 'a1b2c3d4f5g6', false, current_timestamp, null, 'cc2b5506-83ed-5764-985e-611ad4ce8050', 'abc123456'
+insert into users (email, password, admin, created_at, inactivated_at, email_uuid, reset_token, last_password_change)
+select 'testuuid@domain.com', 'a1b2c3d4f5g6', false, current_timestamp, null, 'cc2b5506-83ed-5764-985e-611ad4ce8050', 'abc123456', current_timestamp
 where not exists (select 1 from users where email = 'testuuid@domain.com');