Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces password-protected URL functionality to the Delibird link shortener service. Users can now secure their short links with a passphrase, requiring authentication before redirecting to the destination URL. The implementation uses a challenge-response authentication mechanism with SHA-256 hashing and time-limited nonces stored in DynamoDB to prevent replay attacks.
Key changes:
- Added DynamoDB table for storing authentication nonces with TTL-based expiration
- Implemented password authentication flow with client-side SHA-256 challenge generation
- Extended admin portal UI to support creating and editing password-protected links
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
modules/aws_dynamodb/table.tf |
Creates new DynamoDB table for storing authentication nonces with TTL |
modules/aws_dynamodb/output.tf |
Exposes the nonce table resource for use in other modules |
modules/aws_iam/lambda_redirect_request.tf |
Adds IAM permissions for Lambda to read/write nonce table |
modules/aws_iam/lambda_admin_portal.tf |
Grants admin portal permission to update passphrase attribute |
modules/aws_iam/variables.tf |
Adds nonce table variable for IAM module |
modules/aws_lambda/variables.tf |
Adds variables for nonce table and lifetime configuration |
modules/aws_lambda/function.tf |
Passes nonce table and lifetime as Lambda environment variables |
modules/aws_s3/stylesheet.tf |
Uploads new CSS file for password-protected error page |
s3/css/error-protected.css |
Styles for the password authentication page |
s3/css/admin-portal.css |
Styles for password field toggle buttons in admin forms |
lambda/redirect_request/static/protected.html |
HTML template for password authentication page with client-side challenge generation |
lambda/redirect_request/protected_util.py |
Utility functions for generating protected responses and creating nonces |
lambda/redirect_request/models/delibird_nonce.py |
DynamoDB model for nonce storage and validation |
lambda/redirect_request/query.py |
Adds blacklist filtering to remove authentication parameters from redirected URLs |
lambda/redirect_request/app.py |
Implements authentication flow for protected links |
lambda/layers/common/python/ddb/models/delibird_link.py |
Adds passphrase field and challenge validation to link model |
lambda/layers/common/python/util/response_util.py |
Adds bootstrap-icons support for CSP headers |
lambda/layers/common/python/util/parse_util.py |
Adds query parameter parsing utility function |
lambda/layers/common/python/util/logger_util.py |
Adds development logger setup for PynamoDB debugging |
lambda/admin_portal/static/links.html |
Adds password field UI elements to create/edit link modals |
lambda/admin_portal/portal_page/link_create.py |
Handles passphrase field in link creation requests |
lambda/admin_portal/portal_page/link_update.py |
Handles passphrase field in link update requests |
lambda/admin_portal/app.py |
Enables development logging |
environments/sample/main.tf |
Configures nonce table and lifetime settings for the environment |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def mark_used(self) -> tuple[bool, Optional[datetime]]: | ||
| if not self.is_active(): | ||
| # usedな状態でmark_usedされていないか、呼び出し時点で期限切れしていないか | ||
| return False, None | ||
|
|
||
| now = get_jst_datetime_now() | ||
| try: | ||
| self.update( | ||
| actions=[DelibirdNonceTableModel.used_at.set(now)], | ||
| condition=(DelibirdNonceTableModel.used_at.does_not_exist()) | | ||
| (Path(DelibirdNonceTableModel.used_at).is_type(NULL)) | ||
| ) | ||
| except UpdateError as e: | ||
| if e.cause_response_code == "ConditionalCheckFailedException": | ||
| return False, None | ||
| raise e | ||
| return True, now |
There was a problem hiding this comment.
There's a race condition in the mark_used method. The is_active check at line 39 and the database update at line 45 are not atomic. Between these two operations, another concurrent request could mark the nonce as used, but this check would still pass. While the conditional update at line 45-48 does provide protection, the is_active check should also verify the expiration timestamp again in the database condition to ensure consistency.
No description provided.