Repository navigation
Conversation
What was broken Anonymous visitors to a direct terms link could use acceptance controls and receive a success message after an unauthorized request. Root cause The Community App terms detail container did not check authentication before rendering electronic acceptance or mounting DocuSign. Its success dialog also treated an agreement error as a completed acceptance. What was changed Require a nonempty member auth token for electronic acceptance and DocuSign. Keep electronic terms publicly readable and offer the existing login flow. Show success only for an authenticated visitor with no agreement failure. Document the container's authentication behavior. Any added/updated tests Add nine regression tests for missing tokens, the login action, authenticated acceptance, unauthorized responses, already agreed terms, DocuSign, and logout. npm run lint and npm run build passed. npm test passed with 462 tests and 141 snapshots; 23 existing tests were skipped. Live visual verification was blocked by an unavailable browser and the existing missing sharp native module when starting the local server.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was broken
Anonymous visitors opening
/challenges/terms/detail/:termIdcould use acceptance controls and receive a success message after an unauthorized request.Root cause
The Community App terms detail container did not check authentication before rendering electronic acceptance or mounting DocuSign. Its success dialog also treated an agreement error as a completed acceptance. The Terms API already requires authentication for signing.
What was changed
Files changed:
src/shared/containers/terms-detail/index.jsx__tests__/shared/containers/terms-detail/index.jsxAny added/updated tests
Added nine regression tests covering missing tokens, the login action, authenticated acceptance, unauthorized responses, already agreed terms, DocuSign, and logout.
Validation under Node 10.24.1 after
nvm use:npm run lintpassed, including JavaScript and SCSS.npm run buildpassed, including the retired CMS target check.npm run jest -- --runTestsByPath __tests__/shared/containers/terms-detail/index.jsx --coverage=falsepassed all nine regression tests.npm testpassed: 169 suites, 462 tests, and 141 snapshots. The project skipped four suites and 23 tests.git diff --checkpassed.Live visual verification was blocked: no browser is connected, and the existing local dependency installation lacks
sharp/build/Release/sharp.node, preventingnpm start. Component regression tests verify public reading and authenticated-only signing behavior for the direct-link flow shown in the attachment.Jira: https://topcoder.atlassian.net/browse/PM-6066