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

chore(core): move kratos e2e tests and delete e2e dir #3943

Merged
merged 21 commits into from
Feb 7, 2024
Merged

Conversation

vindard
Copy link
Contributor

@vindard vindard commented Feb 6, 2024

No description provided.

Copy link

gitguardian bot commented Feb 6, 2024

⚠️ GitGuardian has uncovered 12 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
8533381 Triggered Generic High Entropy Secret 1cb8283 core/api/test/unit/domain/authentication/index.spec.ts View secret
8533381 Triggered Generic High Entropy Secret 1cb8283 core/api/test/e2e/servers/kratos.spec.ts View secret
8533381 Triggered Generic High Entropy Secret ca20f97 core/api/test/unit/services/kratos/identity.spec.ts View secret
8533381 Triggered Generic High Entropy Secret ca20f97 core/api/test/unit/domain/authentication/index.spec.ts View secret
8533382 Triggered Generic High Entropy Secret 1cb8283 core/api/test/e2e/servers/kratos.spec.ts View secret
8533382 Triggered Generic High Entropy Secret 1cb8283 core/api/test/e2e/servers/kratos.spec.ts View secret
8533382 Triggered Generic High Entropy Secret 1cb8283 core/api/test/unit/domain/authentication/index.spec.ts View secret
8533382 Triggered Generic High Entropy Secret 1cb8283 core/api/test/unit/domain/authentication/index.spec.ts View secret
8533382 Triggered Generic High Entropy Secret ca20f97 core/api/test/unit/domain/authentication/index.spec.ts View secret
8533382 Triggered Generic High Entropy Secret ca20f97 core/api/test/unit/services/kratos/identity.spec.ts View secret
8533382 Triggered Generic High Entropy Secret ca20f97 core/api/test/unit/services/kratos/identity.spec.ts View secret
8533382 Triggered Generic High Entropy Secret ca20f97 core/api/test/unit/domain/authentication/index.spec.ts View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

core/api/test/integration/services/auth-service.spec.ts Dismissed Show dismissed Hide dismissed
core/api/test/integration/services/auth-service.spec.ts Dismissed Show dismissed Hide dismissed
core/api/test/integration/services/auth-service.spec.ts Dismissed Show dismissed Hide dismissed
core/api/test/integration/services/auth-service.spec.ts Dismissed Show dismissed Hide dismissed
core/api/test/integration/services/auth-service.spec.ts Dismissed Show dismissed Hide dismissed
core/api/test/integration/services/auth-service.spec.ts Dismissed Show dismissed Hide dismissed
@vindard vindard force-pushed the move-kratos-e2e-2 branch 3 times, most recently from b02b98e to ca09d52 Compare February 6, 2024 15:26
@vindard vindard added the core label Feb 6, 2024
@github-actions github-actions bot removed the core label Feb 6, 2024
@github-actions github-actions bot added ci and removed admin-panel labels Feb 6, 2024
@vindard vindard force-pushed the move-kratos-e2e-2 branch 5 times, most recently from 98f7fbc to d6c3899 Compare February 7, 2024 16:23
@github-actions github-actions bot removed the ci label Feb 7, 2024
@@ -87,7 +87,7 @@ export LOG_LEVEL="info"

export KRATOS_MASTER_USER_PASSWORD="passwordHardtoFindWithNumber123"
export KRATOS_PG_HOST="localhost"
export KRATOS_PG_PORT="5433"
export KRATOS_PG_PORT="5432"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont see changes in docker compose.. are you sure about this?

Copy link
Contributor Author

@vindard vindard Feb 7, 2024

Choose a reason for hiding this comment

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

Oh right, this assumes we now target tilt deps and not docker-compose deps anymore. It's probably irrelevant since we're planning to find a way to remove .env and have all the things we would normally need for local dev come through buck either way.

We have some cleaning up of docs, deps, envs and local workflows to do after this PR

@@ -88,7 +88,7 @@ export LOG_LEVEL="info"
export KRATOS_MASTER_USER_PASSWORD="passwordHardtoFindWithNumber123"
export KRATOS_ADMIN_URL="http://localhost:4434"
export KRATOS_PG_HOST="localhost"
export KRATOS_PG_PORT="5433"
export KRATOS_PG_PORT="5432"
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -35,6 +35,20 @@ export const getSupportedCountries = ({
return countries
}

export const getNextPageToken = (link: string): string | undefined => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is specific to service implementation, should not be in the core domain

Copy link
Contributor Author

@vindard vindard Feb 7, 2024

Choose a reason for hiding this comment

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

Moved ca20f97

@@ -24,21 +24,39 @@ type IdentityPhone = IdentityBase & {
phone: PhoneNumber
email: undefined
emailVerified: undefined

username?: undefined
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe I am missing something but kratos is not updated when the user sets the username.. so do we have a process syncing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is different. It's added to IdentityPhone/IdentityEmail/IdentityPhoneEmail just to satisfy the AnyIdentity interface and it really only has meaning in IdentityDeviceAccount where the type is IdentityUsername and not our user's account usernames

@vindard vindard merged commit 5cabf6b into main Feb 7, 2024
11 of 12 checks passed
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