Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v0.4.0 – rewrite: use Config and Secret Stores, enable local development #39

Merged
merged 21 commits into from
Jul 3, 2024

Conversation

doramatadora
Copy link
Contributor

@doramatadora doramatadora commented Jul 2, 2024

Starter kit rewrite

This PR:

  1. Moves all OAuth configuration to Config Store (OIDC discovery metadata, JWKs, others) and Secret Store (client ID and other secrets). This addresses need to redeploy regularly every few months, it seems the key ids change in jwks.json #18
  2. Introduces fastly.toml setup configuration in order to remove the need for setup.sh.
  3. Introduces fastly.toml local server configuration in order to enable out-of-the-box local development, pre-configured for Google OAuth clients.
  4. Removes an obsolete dependency: toml; supersedes Bump toml from 0.5.11 to 0.8.13 #30.
  5. Removes local configuration files.
  6. Updates all dependencies to their latest versions; supersedes Bump serde_json from 1.0.117 to 1.0.119 #38, Bump jwt-simple from 0.11.9 to 0.12.9 #37.
  7. Improves the README.
  8. Updates the CI workflow.

@doramatadora doramatadora requested review from a team as code owners July 2, 2024 17:07
@doramatadora doramatadora requested review from acfoltzer and harmony7 and removed request for a team July 2, 2024 17:07
@doramatadora doramatadora changed the title v0.4.0 – use Config and Secret Stores v0.4.0 – refactor: use Config and Secret Stores, enable local development Jul 2, 2024
Copy link
Member

@harmony7 harmony7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is well written and easy to understand.
I have a couple of questions/comments about it, but in general I think it looks good!

My main complaint about this PR is that you called it a "refactor"; to me what you made here counts more as a next version or a rewrite. =)

refactor: To rewrite existing source code in order to improve its readability, reusability or structure without affecting its meaning or behaviour.

src/cookies.rs Outdated
const COOKIE_PREFIX: &str = "__Secure-";

// Change to "__Secure-" for production, and "local-" for local development.
const COOKIE_PREFIX: &str = "local-";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if you can do something like you do with the redirect_uri (check fastly service version) so that you don't have to modify the code before deploying prod?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestion, thank you!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved

### Deploy the Fastly service and get a domain
### 3. Deploy the Fastly service and get a domain

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to mention that the COOKIE_PREFIX needs to be changed between local and production. Or, modify the code that uses the cookie prefix to automatically use the right prefix, see my comment in cookies.rs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've modified the code so that it automatically uses the right prefix :)

Co-authored-by: Katsuyuki Omuro <[email protected]>
Co-authored-by: Katsuyuki Omuro <[email protected]>
@doramatadora doramatadora changed the title v0.4.0 – refactor: use Config and Secret Stores, enable local development v0.4.0 – rewrite: use Config and Secret Stores, enable local development Jul 3, 2024
@doramatadora doramatadora merged commit a584525 into main Jul 3, 2024
3 checks passed
@doramatadora doramatadora deleted the dora-use-stores branch July 3, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants