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

#2035 - migration vers proconnect partie 2 #2161

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

celineung
Copy link
Contributor

No description provided.

Copy link

socket-security bot commented Sep 9, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@codegouvfr/react-dsfr@1.13.2 None 0 80.9 MB garronej

🚮 Removed packages: npm/@codegouvfr/react-dsfr@1.10.11

View full report↗︎

@celineung celineung force-pushed the 2035-migration-vers-proconnect-part2 branch from cfa3df7 to 710bfb3 Compare September 10, 2024 14:59
@enguerranws enguerranws force-pushed the 2035-migration-vers-proconnect-part2 branch from 3b79f91 to e6e054c Compare September 11, 2024 10:19
@celineung celineung force-pushed the 2035-migration-vers-proconnect-part2 branch from e6e054c to 6e67aab Compare September 11, 2024 14:11
@enguerranws enguerranws force-pushed the 2035-migration-vers-proconnect-part2 branch from 6e67aab to 7df3a15 Compare September 12, 2024 09:56
@celineung celineung force-pushed the 2035-migration-vers-proconnect-part2 branch 2 times, most recently from a18ddfc to 7f79fd3 Compare September 16, 2024 13:55
@celineung celineung changed the base branch from 2035-migration-vers-proconnect to main September 16, 2024 13:57
@@ -7,17 +8,25 @@ export type FederatedIdentityProvider =
export const federatedIdentityProviders = [
"inclusionConnect",
"peConnect",
"proConnect",
Copy link
Contributor

Choose a reason for hiding this comment

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

On ne l'utilise pas finalement, à prévoir en renaming une fois que tout sera migré.

shared/src/errors/errors.ts Outdated Show resolved Hide resolved
return state$.value.auth.federatedIdentityWithUser?.provider ===
"peConnect"
? inclusionConnectedGateway.getLogoutUrl$({
idToken: "",
Copy link
Contributor

Choose a reason for hiding this comment

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

Deconnexion de PE Connect à tester

Copy link
Contributor

Choose a reason for hiding this comment

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

Refacto de cette partie

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅ refacto

back/src/domains/convention/use-cases/GetConvention.ts Outdated Show resolved Hide resolved
back/src/domains/convention/use-cases/GetConvention.ts Outdated Show resolved Hide resolved
Copy link

Comment on lines +51 to +56
new AppConfigBuilder({
INCLUSION_CONNECT_GATEWAY: "IN_MEMORY",
INCLUSION_CONNECT_CLIENT_SECRET: fakeProviderConfig.clientSecret,
INCLUSION_CONNECT_CLIENT_ID: fakeProviderConfig.clientId,
INCLUSION_CONNECT_BASE_URI: fakeProviderConfig.providerBaseUri,
DOMAIN: immersionDomain,
Copy link
Collaborator

Choose a reason for hiding this comment

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

on a pas de tests avec pro connect en e2e ?

Comment on lines +362 to +363
external_id_inclusion_connect: string | null;
external_id_pro_connect: string | null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

vous preferez pas virer le external ? :

Suggested change
external_id_inclusion_connect: string | null;
external_id_pro_connect: string | null;
inclusion_connect_sub: string | null;
pro_connect_sub: string | null;

Comment on lines +6 to +7
const newInclusionConnectExternalIdColumnName = "external_id_inclusion_connect";
const proConnectExternalIdColumnName = "external_id_pro_connect";
Copy link
Collaborator

Choose a reason for hiding this comment

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

idem, pour le nommage

authPayload,
uow,
convention,
provider: provider,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
provider: provider,
provider,

authPayload,
uow,
convention,
provider: provider,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
provider: provider,
provider,

const existingOngoingOAuth = await uow.ongoingOAuthRepository.findByState(
params.state,
"inclusionConnect",
const mode = oAuthProviderByFeatureFlags(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const mode = oAuthProviderByFeatureFlags(
const provider = oAuthProviderByFeatureFlags(

Comment on lines +127 to +133
const toto = this.#makeAuthenticatedUser(
this.#uuidGenerator.new(),
this.#timeGateway.now(),
payload,
);
const newOrUpdatedAuthenticatedUser: User = {
...this.#makeAuthenticatedUser(
this.#uuidGenerator.new(),
this.#timeGateway.now(),
icIdTokenPayload,
),
...toto,
Copy link
Collaborator

Choose a reason for hiding this comment

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

variable a renommer ou à in line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙈

) {
super();
super(uowPerformer);
Copy link
Collaborator

Choose a reason for hiding this comment

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

je crois que c'est pas obligatoire ça....
Une autre option est d'utiliser : createTransactionalUseCase

Comment on lines +99 to +100
external_id_inclusion_connect: user.externalId, // Attention ici c'est pour tester le script de mig
external_id_pro_connect: null, // Il faudra inverser une fois migré
Copy link
Collaborator

Choose a reason for hiding this comment

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

commentaire toujours utile ? 

map((logoutUrl) => {
navigationGateway.goToUrl(logoutUrl);
if (logoutUrl) navigationGateway.goToUrl(logoutUrl);
return authSlice.actions.loggedOutSuccessfullyFromInclusionConnect();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return authSlice.actions.loggedOutSuccessfullyFromInclusionConnect();
return authSlice.actions.loggedOutSuccessfullyFromProvider();

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.

4 participants