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

feat: generated Permit client, project permission logic #1699

Merged
merged 15 commits into from
Mar 28, 2024

Conversation

jonaro00
Copy link
Member

@jonaro00 jonaro00 commented Mar 21, 2024

Adds Permit logic to the project create+delete actions. This will be released without taking control of permission checking to allow a background migration between releases.

Things for upcoming PRs:

  • Implement organizations (commented parts) in the permit client + tests
  • Command in gateway to trigger a sync of project data
  • Let Permit take over the Permission checks in ScopedUser
  • Add CORS + update Console

@jonaro00 jonaro00 force-pushed the permit-openapi-client branch 2 times, most recently from f47320e to 8e8ee8c Compare March 25, 2024 20:36
@jonaro00 jonaro00 changed the title feat: use generated Permit openapi client feat: generated Permit client, project permission logic Mar 26, 2024
@jonaro00 jonaro00 force-pushed the permit-openapi-client branch from fdbd629 to fab937c Compare March 26, 2024 20:19
@jonaro00 jonaro00 force-pushed the permit-openapi-client branch from fab937c to 1ef4349 Compare March 26, 2024 20:23
@jonaro00 jonaro00 marked this pull request as ready for review March 26, 2024 21:35
@@ -81,7 +83,9 @@ pub async fn sync(pool: PgPool, args: SyncArgs) -> io::Result<()> {
}
}
}
Err(shuttle_backends::client::Error::RequestError(StatusCode::NOT_FOUND)) => {
Err(_) => {
// FIXME: Make the error type better so that this is only done on 404s
Copy link
Member Author

Choose a reason for hiding this comment

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

This is fixed in #1705

Copy link
Contributor

@iulianbarbu iulianbarbu left a comment

Choose a reason for hiding this comment

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

The permit client changes and some of the tests are heavy to go through in this review. It is probably best to be reviewed by @chesedo who has more context, but apart from this, left a few qs for now.

@@ -68,7 +68,9 @@ services:
- "--stripe-secret-key=${STRIPE_SECRET_KEY}"
# used only for local development
- "--jwt-signing-private-key=LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1DNENBUUF3QlFZREsyVndCQ0lFSUR5V0ZFYzhKYm05NnA0ZGNLTEwvQWNvVUVsbUF0MVVKSTU4WTc4d1FpWk4KLS0tLS1FTkQgUFJJVkFURSBLRVktLS0tLQo="
- "--permit-api=https://api.eu-central-1.permit.io"
- "--permit-api-uri=https://api.eu-central-1.permit.io"
Copy link
Contributor

Choose a reason for hiding this comment

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

I imagine this can differ based on how a Permit account is configured. Makes me think that it is better to hide it behind an env var.

@jonaro00 jonaro00 force-pushed the permit-openapi-client branch from 18d5cad to 0009469 Compare March 27, 2024 17:16
@jonaro00 jonaro00 force-pushed the permit-openapi-client branch from 0009469 to 02cbccc Compare March 27, 2024 17:23
@jonaro00 jonaro00 merged commit e33329b into main Mar 28, 2024
30 of 32 checks passed
@jonaro00 jonaro00 deleted the permit-openapi-client branch March 28, 2024 09:40
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