#AMM-1247- feat: VAPT Captcha implementation#81
Conversation
WalkthroughA CAPTCHA mechanism has been integrated into the application's login process. This includes a new Angular CAPTCHA component and service, updates to the login form and authentication logic to require and validate a CAPTCHA token, and corresponding environment variable and configuration updates for site key, challenge URL, and enable flag support. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginComponent
participant CaptchaComponent
participant CaptchaService
participant AuthenticationService
participant Backend
User->>LoginComponent: Enter username/password
LoginComponent->>CaptchaComponent: Render CAPTCHA
CaptchaComponent->>CaptchaService: Load CAPTCHA script
CaptchaService-->>CaptchaComponent: Script loaded
User->>CaptchaComponent: Solve CAPTCHA
CaptchaComponent->>LoginComponent: Emit tokenResolved(token)
User->>LoginComponent: Click Login
LoginComponent->>AuthenticationService: login(username, password, ..., captchaToken)
AuthenticationService->>Backend: POST login request with captchaToken
Backend-->>AuthenticationService: Respond (success/failure)
AuthenticationService-->>LoginComponent: Return result
LoginComponent->>CaptchaComponent: (On error) Reset CAPTCHA
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (8)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
src/environments/environment.local.ts (1)
170-171: Consider providing test values for local development.Empty CAPTCHA configuration in the local environment might make it difficult to test CAPTCHA functionality during development. Consider using test/sandbox keys for local development.
-const siteKey = ''; -const captchaChallengeURL = ''; +const siteKey = '1x00000000000000000000AA'; // Cloudflare test key +const captchaChallengeURL = 'https://challenges.cloudflare.com/turnstile/v0/api.js';src/app/login/login.component.css (1)
73-78: Consider alternatives to !important declarations.While the disabled button styling is functionally correct, the use of
!importantdeclarations suggests potential specificity issues. Consider using more specific selectors or CSS modules to avoid overriding styles with!important..disabledLoginButton { background-color: #ccc; color: #666; cursor: not-allowed; box-shadow: none; }If the
!importantdeclarations are necessary to override framework styles, consider adding a comment explaining why they're required.src/environments/environment.ci.ts.template (1)
156-156: Add trailing comma for consistency.Consider adding a trailing comma after
captchaChallengeURLto maintain consistency with the codebase style and make future additions easier.siteKey: siteKey, captchaChallengeURL: captchaChallengeURL, };src/app/app-modules/core/components/captcha/captcha.component.ts (2)
14-14: Improve type safety for turnstile APIUsing
anytype reduces type safety and IDE support. Consider creating a proper interface for the turnstile API.-declare const turnstile: any; +interface TurnstileAPI { + render(element: HTMLElement, options: { + sitekey: string; + theme: string; + callback: (token: string) => void; + }): string; + reset(widgetId: string): void; + remove?(widgetId: string): void; +} + +declare const turnstile: TurnstileAPI;
49-53: Add safety checks to reset methodThe reset method should include additional validation to prevent runtime errors.
public reset() { - if (this.widgetId && typeof turnstile !== 'undefined') { + if (this.widgetId && typeof turnstile !== 'undefined' && turnstile.reset) { turnstile.reset(this.widgetId); + } else if (!this.widgetId) { + console.warn('CAPTCHA widget not initialized, cannot reset'); } }src/app/login/login.component.ts (2)
35-35: Improve type safety for ViewChild declaration.The ViewChild is typed as
anywhich reduces type safety. Consider creating a proper interface or importing the specific captcha component type.- @ViewChild('captchaCmp') captchaCmp: any; + @ViewChild('captchaCmp') captchaCmp!: CaptchaComponent;Ensure you import the CaptchaComponent type at the top of the file.
331-336: Enhance error handling in resetCaptcha method.The method has good type checking for the reset function but could be more robust in handling edge cases.
resetCaptcha() { - if (this.captchaCmp && typeof this.captchaCmp.reset === 'function') { + if (this.captchaCmp?.reset && typeof this.captchaCmp.reset === 'function') { this.captchaCmp.reset(); this.captchaToken = ''; + } else { + // Fallback: just clear the token if component is not available + this.captchaToken = ''; } }This ensures the token is always cleared even if the captcha component is unavailable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
scripts/ci-prebuild.js(1 hunks)src/app/app-modules/core/components/captcha/captcha.component.html(1 hunks)src/app/app-modules/core/components/captcha/captcha.component.ts(1 hunks)src/app/app-modules/core/core.module.ts(4 hunks)src/app/app-modules/core/services/captcha.service.ts(1 hunks)src/app/login/authentication.service.ts(1 hunks)src/app/login/login.component.css(1 hunks)src/app/login/login.component.html(3 hunks)src/app/login/login.component.ts(8 hunks)src/environments/environment.ci.ts.template(2 hunks)src/environments/environment.development.ts(2 hunks)src/environments/environment.local.ts(2 hunks)src/environments/environment.prod.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/app/login/authentication.service.ts (3)
src/environments/environment.development.ts (1)
environment(44-164)src/environments/environment.local.ts (1)
environment(52-172)src/environments/environment.prod.ts (1)
environment(46-166)
🔇 Additional comments (15)
src/environments/environment.local.ts (1)
49-50: LGTM! Configuration follows established patterns.The CAPTCHA configuration constants are properly declared and follow the existing naming conventions.
src/app/app-modules/core/components/captcha/captcha.component.html (1)
1-1: LGTM! Clean and appropriate CAPTCHA container implementation.The template properly provides a container for the Cloudflare Turnstile widget with appropriate ID and template reference for component access.
src/environments/environment.development.ts (1)
42-43: Configuration follows established patterns.The CAPTCHA constants are properly declared following the existing environment configuration structure.
src/environments/environment.prod.ts (2)
43-44: Verify CAPTCHA configuration for production deployment.The
siteKeyandcaptchaChallengeURLare initialized as empty strings, which will prevent CAPTCHA functionality from working in production. Ensure these values are properly configured during the deployment process.Consider implementing a build-time validation to ensure these critical security values are not empty in production builds.
164-165: CAPTCHA environment configuration added correctly.The new CAPTCHA properties are properly integrated into the environment object and follow the established pattern.
src/app/login/login.component.css (1)
59-70: Excellent accessibility implementation.The
.sr-onlyclass follows standard web accessibility patterns, providing content for screen readers while visually hiding it. This enhances the user experience for assistive technologies.src/environments/environment.ci.ts.template (1)
33-34: CAPTCHA template configuration implemented correctly.The template placeholders
<%= SITE_KEY %>and<%= CAPTCHA_CHALLENGE_URL %>follow the established pattern and are properly integrated into the environment object.Also applies to: 155-156
src/app/app-modules/core/core.module.ts (1)
89-90: LGTM: Proper Angular module registrationThe CAPTCHA component and service are correctly registered in the CoreModule following Angular conventions. The component is properly declared and exported, while the service is provided as a singleton through the
forRoot()method.Also applies to: 153-153, 186-186, 204-204
src/app/login/login.component.html (3)
16-16: LGTM: Enhanced form validationAdding the
requiredattribute to username and password fields provides immediate client-side validation feedback.Also applies to: 34-34
58-65: Excellent accessibility implementationThe CAPTCHA integration includes proper ARIA attributes with
role="group",aria-labelledby, and a screen-reader accessible label. The centered styling provides good visual presentation.
73-76: LGTM: Proper conditional button stateThe login button correctly requires both valid form input and a completed CAPTCHA token. The disabled state and CSS class provide clear visual feedback to users.
src/app/login/login.component.ts (4)
22-22: LGTM: Proper import for ViewChild decorator.The ViewChild import is correctly added to support the captcha component reference.
149-154: LGTM: Consistent captcha token parameter.The second login call correctly includes the captchaToken parameter, maintaining consistency with the initial login attempt.
169-169: LGTM: Proper captcha reset on error scenarios.The captcha is appropriately reset in all error handling paths, preventing reuse of expired tokens and ensuring fresh captcha challenges for subsequent login attempts.
Also applies to: 176-176, 184-184, 198-198, 205-205
327-329: LGTM: Clean captcha resolution handler.The method properly captures the resolved captcha token for use in login requests.
|
2 participants
📋 Description
JIRA ID: AMM-1247
This feature is part of VAPT and includes captcha implementation, includes validators for login form
✅ Type of Change
Summary by CodeRabbit