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: refactored cloud auth #317

Merged
merged 2 commits into from
Apr 17, 2024
Merged

feat: refactored cloud auth #317

merged 2 commits into from
Apr 17, 2024

Conversation

chris13524
Copy link
Member

@chris13524 chris13524 commented Apr 16, 2024

Description

Uses the new Cloud authentication mechanism introduced in this PR. Slack conversation

Instead of a Supabase user JWT being provided to Push Server, Push Server authenticating this JWT with a shared secret from Cloud, and then Push Server checking with Push Server if the user has access to the tenant... now the authorization of the user's access to the project/tenant is completely offloaded to Cloud and all Push Server does now is checks a JWT signature. This JWT is now signed by Cloud app with a different shared secret and has a TTL of 30s rather than being the same one used for all user authentication in Cloud.

Resolves that Cloud currently is not able to configure Push Server due to a breaking change that was overlooked.

Remaining work:

  • Update JWT_SECRET to new secret
  • Remove CLOUD_API_SECRET secret

How Has This Been Tested?

Not tested

Due Diligence

  • Breaking change
  • Requires a documentation update
  • Requires a e2e/integration test update

@chris13524 chris13524 self-assigned this Apr 16, 2024
@arein arein added the accepted The issue has been accepted into the project label Apr 16, 2024
Copy link
Contributor

Show Plan

[command]/home/runner/work/_temp/91d37353-214c-4dec-8b29-3b686567731e/terraform-bin -chdir=terraform show -no-color /tmp/plan.tfplan

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # module.ecs.aws_ecs_service.app_service will be updated in-place
  ~ resource "aws_ecs_service" "app_service" {
        id                                 = "arn:aws:ecs:eu-central-1:898587786287:service/staging-push/staging-push-service"
        name                               = "staging-push-service"
        tags                               = {}
      ~ task_definition                    = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push:170" -> (known after apply)
        # (15 unchanged attributes hidden)

        # (4 unchanged blocks hidden)
    }

  # module.ecs.aws_ecs_task_definition.app_task_definition must be replaced
-/+ resource "aws_ecs_task_definition" "app_task_definition" {
      ~ arn                      = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push:170" -> (known after apply)
      ~ arn_without_revision     = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push" -> (known after apply)
      ~ container_definitions    = (sensitive value) # forces replacement
      ~ id                       = "staging-push" -> (known after apply)
      ~ revision                 = 170 -> (known after apply)
      - tags                     = {} -> null
        # (11 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Plan: 1 to add, 1 to change, 1 to destroy.
::debug::Terraform exited with code 0.
::debug::stdout: %0ATerraform used the selected providers to generate the following execution%0Aplan. Resource actions are indicated with the following symbols:%0A  ~ update in-place%0A-/+ destroy and then create replacement%0A%0ATerraform will perform the following actions:%0A%0A  # module.ecs.aws_ecs_service.app_service will be updated in-place%0A  ~ resource "aws_ecs_service" "app_service" {%0A        id                                 = "arn:aws:ecs:eu-central-1:898587786287:service/staging-push/staging-push-service"%0A        name                               = "staging-push-service"%0A        tags                               = {}%0A      ~ task_definition                    = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push:170" -> (known after apply)%0A        # (15 unchanged attributes hidden)%0A%0A        # (4 unchanged blocks hidden)%0A    }%0A%0A  # module.ecs.aws_ecs_task_definition.app_task_definition must be replaced%0A-/+ resource "aws_ecs_task_definition" "app_task_definition" {%0A      ~ arn                      = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push:170" -> (known after apply)%0A      ~ arn_without_revision     = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push" -> (known after apply)%0A      ~ container_definitions    = (sensitive value) # forces replacement%0A      ~ id                       = "staging-push" -> (known after apply)%0A      ~ revision                 = 170 -> (known after apply)%0A      - tags                     = {} -> null%0A        # (11 unchanged attributes hidden)%0A%0A        # (1 unchanged block hidden)%0A    }%0A%0APlan: 1 to add, 1 to change, 1 to destroy.%0A
::debug::stderr: 
::debug::exitcode: 0

Action: pull_request

@@ -149,6 +140,9 @@ pub async fn handler(
return Err(e);
}

// Ensure tenant real
let _existing_tenant = state.tenant_store.get_tenant(&id).await?;
Copy link
Member Author

Choose a reason for hiding this comment

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

Moving after, auth comes first

@chris13524 chris13524 marked this pull request as ready for review April 16, 2024 16:41
Copy link
Contributor

Show Plan

[command]/home/runner/work/_temp/ccd426a2-df9c-42d6-a1c4-4fc7f0b555aa/terraform-bin -chdir=terraform show -no-color /tmp/plan.tfplan

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # module.ecs.aws_ecs_service.app_service will be updated in-place
  ~ resource "aws_ecs_service" "app_service" {
        id                                 = "arn:aws:ecs:eu-central-1:898587786287:service/staging-push/staging-push-service"
        name                               = "staging-push-service"
        tags                               = {}
      ~ task_definition                    = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push:170" -> (known after apply)
        # (15 unchanged attributes hidden)

        # (4 unchanged blocks hidden)
    }

  # module.ecs.aws_ecs_task_definition.app_task_definition must be replaced
-/+ resource "aws_ecs_task_definition" "app_task_definition" {
      ~ arn                      = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push:170" -> (known after apply)
      ~ arn_without_revision     = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push" -> (known after apply)
      ~ container_definitions    = (sensitive value) # forces replacement
      ~ id                       = "staging-push" -> (known after apply)
      ~ revision                 = 170 -> (known after apply)
      - tags                     = {} -> null
        # (11 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Plan: 1 to add, 1 to change, 1 to destroy.
::debug::Terraform exited with code 0.
::debug::stdout: %0ATerraform used the selected providers to generate the following execution%0Aplan. Resource actions are indicated with the following symbols:%0A  ~ update in-place%0A-/+ destroy and then create replacement%0A%0ATerraform will perform the following actions:%0A%0A  # module.ecs.aws_ecs_service.app_service will be updated in-place%0A  ~ resource "aws_ecs_service" "app_service" {%0A        id                                 = "arn:aws:ecs:eu-central-1:898587786287:service/staging-push/staging-push-service"%0A        name                               = "staging-push-service"%0A        tags                               = {}%0A      ~ task_definition                    = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push:170" -> (known after apply)%0A        # (15 unchanged attributes hidden)%0A%0A        # (4 unchanged blocks hidden)%0A    }%0A%0A  # module.ecs.aws_ecs_task_definition.app_task_definition must be replaced%0A-/+ resource "aws_ecs_task_definition" "app_task_definition" {%0A      ~ arn                      = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push:170" -> (known after apply)%0A      ~ arn_without_revision     = "arn:aws:ecs:eu-central-1:898587786287:task-definition/staging-push" -> (known after apply)%0A      ~ container_definitions    = (sensitive value) # forces replacement%0A      ~ id                       = "staging-push" -> (known after apply)%0A      ~ revision                 = 170 -> (known after apply)%0A      - tags                     = {} -> null%0A        # (11 unchanged attributes hidden)%0A%0A        # (1 unchanged block hidden)%0A    }%0A%0APlan: 1 to add, 1 to change, 1 to destroy.%0A
::debug::stderr: 
::debug::exitcode: 0

Action: pull_request

Copy link
Contributor

@Cali93 Cali93 left a comment

Choose a reason for hiding this comment

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

Nice one! One thing that we could look into in a separate PR is to using staging registry for project id validation, like Notify Server does.

@chris13524 chris13524 merged commit 830ad97 into main Apr 17, 2024
9 checks passed
@chris13524 chris13524 deleted the feat/refactored-cloud-auth branch April 17, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted The issue has been accepted into the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants