Conversation
c34581f to
0082eef
Compare
2chanhaeng
left a comment
2chanhaeng
left a comment
There was a problem hiding this comment.
First, tests for #81 are needed.
Also, this PR seems to contain more changes related to reducing multiple origins to a single origin than changes related to #81. Wouldn’t it be better to keep the PR and create a separate issue for this? It would also be helpful to include in the issue the reasons for changing to a single origin.
It also seems that the documentation needs to be updated. Searching the codebase for DRFED_LOGIN_ORIGINS, loginOrigins, and similar terms still turns up a lot of outdated documentation.
0a58caf to
812c0c1
Compare
dahlia
left a comment
dahlia
left a comment
There was a problem hiding this comment.
Following up on @2chanhaeng's review: please add a regression test that configures https://app.example. and verifies that a login link on https://app.example is accepted. Cover the CLI configuration path and direct createYogaServer() usage, and keep IP literals allowed. The current test changes only adapt existing tests to the new interface.
I agree that the single-origin restriction belongs in a separate discussion. The reply on #81 explains why multiple origins can be useful.
|
@dahlia Rather than switching from supporting multiple options to supporting only one and then back to supporting multiple options again, I think it would be better to make this PR the final step in switching to supporting multiple options. |
Assited-by Codex: gpt-6.1-sol Prompt `` Fix CLI's readme and dev.mts' comment about --login-origin ```
Assisted-by Codex:gpt-6.1-sol Prompt ``` Make a CLI regression test in login.test.ts. ```
3 participants
Closes: #81
Change:
--login-originoptions for running SeverURLof--login-originand its origin..envto set value, like--root-origin.