Centralise test and demo passwords and guard them against policy changes
まだ誰も着手していません。
評価
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 初心者へのやさしさ
- 35/100
- issue の種類
- リファクタリング
- 明瞭さ
- 明確に書かれている
- 活発さ
- 活発
- 技術スタック
- cypress, typescript
調査の方向性
Start with application/common/src/PasswordStrength.ts, application/common/testing/constants.ts, and the named backend and Cypress examples to map existing policy checks and password literals; coordinate timing with the listed prerequisite issues, which edit the same test files. Then implement the shared password module and guard test described in the issue, including the API drift check if in scope. Done means the acceptance criteria hold and backend, common, and both client test suites pass.
索引モデルが issue の本文から書いたものです。
説明
Follow-up to #899. Best picked up once #899, #910, #927, #937, #948 and #888 have merged, since they all edit the same test files and some add new test passwords.
Every time the password policy changes, someone has to search the repo by hand for test, seed and demo passwords that no longer pass. That search is easy to get wrong, and the misses show up late or not at all.
Problems
Passwords are hand-written all over the test suite. TestUsers in application/common/testing/constants.ts is the only shared home, and it only covers seeded users. Most backend tests and Cypress specs make up their own, for example Loginforbogusperson1 in AuditLogsController.test.ts, Aadsfoswefw1515fd@! in user-client/cypress/e2e/registration.cy.js, and the fillRegistrationForm default in common/cypress/support/commands.ts. The open PRs add a few more, such as Constellation-battery-24! in #927. #910 also adds a second shared value, STRONG_TEST_PASSWORD, next to TestUsers.
Policy changes are found late, or never. #868 raised the minimum length from 8 to 14 and had to touch dozens of test and fixture files. In #899, adding common words meant updating example passwords. The review listed a handful of files and a search turned up more, including the fillRegistrationForm default, which would only have failed in the cross-app Cypress suite after merge. The old TestUsers admin password broke too, and it showed up as failing integration tests rather than pointing at the value.
Seeding itself never breaks, because the seed scripts hash passwords without checking strength. What breaks is any test that types a seeded password into register, reset or setup.
Nothing checks the public demo login. When secret joined the common-word list, README.md, .env.example and the Helm demo-reset job were all advertising a password the app rejects. No test failed. It was caught in review.
Negative tests depend on the order the rules run in. For example, user-client/cypress/e2e/passwordReset.cy.js expects the error for Asdfasdfasdfasdf to start with the "number" rule. If a matching word ever joins the list, the common-word message comes first and the test breaks, even though the password still has no number.
The policy itself is defined in more than one place. The minimum length lives in checkPasswordStrength and in @minLength comments on the request types, which #937 brings down to one Password type. tsoa reads those comments, so they can't reference a constant (#871). commonPasswordBaseWords sits in a test-data file even though production code imports it. The existing length tests send passwords far below the minimum (pass, weak), so they still pass if the API's minimum and checkPasswordStrength drift apart by a few characters.
Suggested solution
1. One module for test and demo passwords. Add application/common/testing/passwords.ts with a valid password, the demo password, and a set of invalid ones built from the policy, each breaking exactly one rule:
export const VALID_PASSWORD = 'Constellation2026' // replaces STRONG_TEST_PASSWORD from #910
export const DEMO_PASSWORD = 'Consentdemo123'
export const INVALID_PASSWORDS = {
tooShort: 'Aa1'.padEnd(PASSWORD_MIN_LENGTH - 1, 'x'),
commonWord: `${VALID_PASSWORD}${commonPasswordBaseWords[0]}`,
noUppercase: VALID_PASSWORD.toLowerCase(),
noLowercase: VALID_PASSWORD.toUpperCase(),
noNumber: VALID_PASSWORD.replace(/\d/g, 'x'),
}
Backend tests, Cypress specs and fillRegistrationForm import from it instead of writing literals. TestUsers stays the home for seeded users. Login tests that only need a wrong password can keep a literal, since login doesn't run checkPasswordStrength. #937 does put @minLength 14 on LoginRequest.password, so those literals still have to meet the minimum length.
2. A guard test. application/common/testing/passwords.test.ts runs in test (common) on every PR and checks that:
- every
TestUserspassword,VALID_PASSWORDandDEMO_PASSWORDpassescheckPasswordStrength. After #910, it also passes each seeded user's name and email - each invalid password fails for exactly its one rule
README.md,.env.exampleand the Helmcron.yamlcontainDEMO_PASSWORD
A PR that adds a common word or changes a rule then fails on its own CI and names the value to change.
3. One home for the policy.
- Export
PASSWORD_MIN_LENGTHfromapplication/common/src/PasswordStrength.tsand movecommonPasswordBaseWordsthere. - Keep
@minLengthon thePasswordtype as a literal, with a//comment pointing atPASSWORD_MIN_LENGTH. - Add a test through the API that catches the two drifting apart, for example
POST /users/password/resetwith an invalid token returning 422 just under the minimum and 403 at it. - Build the "at least 14 characters" text in the Cypress specs from
PASSWORD_MIN_LENGTH.
Parts 1 and 2 belong together, because the guard can only see passwords that live in the module. Part 1 needs the PASSWORD_MIN_LENGTH export from part 3, so that export goes in the first PR. The rest of part 3 can go in the same PR or a second one.
Acceptance criteria
- Test passwords that must pass the policy come from
passwords.tsorTestUsers, apart from the demo-login files and integrations test data - Adding a word used by any fixture to
commonPasswordBaseWordsfails the guard test, naming the fixture - Changing the minimum length in one place but not the other fails the drift test
- Backend, common and both client test suites pass
Considerations
- If #871 (Zod) goes ahead, the schema can use
PASSWORD_MIN_LENGTHdirectly and the drift test goes away. ADR-000 in #937 chose thecommonTypes.tsrefactor over Zod for now. - A CI check for stray password literals is possible but needs an allowlist for deliberate bad passwords. A line in the PR template may be enough to start.
- The docs site lives in
ctrl-docs, so the guard can't check it.
Related: #889, #868, #871, #888, #899, #910, #927, #937, #948
- 主要言語
- TypeScript
- スター
- 4
- フォーク
- 0
- 平均マージ
- 14時間 50分
- マージ済み PR(30日)
- 5
環境構築
- Dockerfile または Docker Compose ファイルあり
- プルリクエストのテンプレートあり
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
Garvan-Data-Science-Platform/ctrl のほかの issue
-
難易度 1/5 1〜3時間 初心者へのやさしさ 82/100
-
admin-client backend bug
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
-
backend bug
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
Garvan-Data-Science-Platform/ctrl#970 · コメント 1 件 ·
-
backend bug
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
-
backend bug
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
Garvan-Data-Science-Platform/ctrl の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 74/100
AvaProtocol/ava-sdk-js#276 ·
メンテナーはふだん 1 日以内に返信
-
useFragment does not require `variables` when the fragment's variables type has required fieldsオープン
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
apollographql/apollo-client#13493 ·
メンテナーはふだん 1 日以内に返信
-
Custom hosts written as trailing-dot FQDNs (`host.local.`) aren't detected as local backends対応中かも @roboomp が今日担当しました。 オープンbug prio:p2 providers triaged
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
can1357/oh-my-pi#15798 · コメント 3 件 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 62/100
code-yeongyu/oh-my-openagent#10172 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100
PDFCraftTool/pdfcraft#189 ·