-
Notifications
You must be signed in to change notification settings - Fork 5
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
[VAS] Story #11619: Multiple organizations on same domain mail #1695
[VAS] Story #11619: Multiple organizations on same domain mail #1695
Conversation
New Issues
|
5c69337
to
7d4ad70
Compare
625012c
to
daa0598
Compare
73fd224
to
50a992f
Compare
67a8565
to
45d5ada
Compare
8637616
to
7dc6645
Compare
88fd5cc
to
a272c6b
Compare
@@ -9,17 +9,17 @@ These components are a set of REST/JSON web services to perform CRUD operations | |||
- profiles | |||
- users. | |||
|
|||
There are composed of the web services themselves (api-iam-server module), the REST clients of these web services (api-iam-client module) and the DTOs shared between the two modules (api-iam-common module). | |||
|
|||
There are composed of the web services themselves (api-iam-server module), the REST clients of these web services ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pourquoi ces sauts de lignes ici ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simple formatage. Markdown ignore les simples hauts
@@ -135,5 +135,6 @@ public class IdentityProviderDto extends CustomerIdDto { | |||
|
|||
private Boolean usePkce; | |||
|
|||
// FIXME : Convert to enum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
À faire maintenant ou créer une story pour pas l'oublier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
à faire plus tard en clean code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Attention à la divergence de code avec la future CAS v7.
checkResponse(response); | ||
} | ||
|
||
public UserDto getUserByEmail(final ExternalHttpContext context, final String email, final Optional<String> embedded) { | ||
// FIXME : getUserByEmail vs getUser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
À faire maintenant ou créer une story pour pas l'oublier.
checkResponse(response); | ||
} | ||
|
||
public UserDto getUserByEmail(final ExternalHttpContext context, final String email, final Optional<String> embedded) { | ||
// FIXME : getUserByEmail vs getUser | ||
@VisibleForTesting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
La documentation de l'annotation VisibleForTesting indique :
Do not use this interface for public or protected declarations: it is a fig leaf for bad design, and it does not prevent anyone from using the declaration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, en plus c'était pas si test only. Je supprime
...external-client/src/main/java/fr/gouv/vitamui/iam/external/client/CasExternalRestClient.java
Show resolved
Hide resolved
...external-client/src/main/java/fr/gouv/vitamui/iam/external/client/CasExternalRestClient.java
Show resolved
Hide resolved
th:href="@{/css/cas.css}"/> | ||
<link rel="stylesheet" type="text/css" href="../../static/icons/vitamui-icons.css" | ||
th:href="@{/icons/vitamui-icons.css}"/> | ||
<script type="text/javascript" src="../../../../../../deployment/roles/reverse/files/apache/page/js/jquery.min.js" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jquery
th:href="@{/css/cas.css}"/> | ||
<link rel="stylesheet" type="text/css" href="../static/icons/vitamui-icons.css" | ||
th:href="@{/icons/vitamui-icons.css}"/> | ||
<script type="text/javascript" src="../../../../../../deployment/roles/reverse/files/apache/page/js/jquery.min.js" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jquery
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ces changements ne devraient-ils pas être faits dans v7.1 plutôt ?
integration-tests/src/test/java/fr/gouv/vitamui/utils/TestConstants.java
Show resolved
Hide resolved
…with internal provider
…t and password update
…anisations with multi domain
a272c6b
to
8c837c0
Compare
Mes retours sont non bloquants, vous pouvez merger, mais il faudra peut-être les traiter par la suite. |
|
||
/** | ||
* Tests {@link IdentityProviderHelper}. | ||
* | ||
* | ||
*/ | ||
public final class IdentityProviderHelperTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je ne vois pas de tests sur les nouvelles méthodes : findAllByUserIdentifier
, findByUserIdentifierAndCustomerId
et identifierMatchProviderPattern
.
...external-client/src/main/java/fr/gouv/vitamui/iam/external/client/CasExternalRestClient.java
Show resolved
Hide resolved
...m-external/src/main/java/fr/gouv/vitamui/iam/external/server/service/CasExternalService.java
Show resolved
Hide resolved
...c/main/java/fr/gouv/vitamui/iam/external/server/service/IdentityProviderExternalService.java
Show resolved
Hide resolved
throw new ApplicationServerException("Unable to update password : customer not found"); | ||
} | ||
User user = findUserByEmailAndCustomerId(email, customerId); | ||
if (UserTypeEnum.NOMINATIVE != user.getType()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exemple de test manquant : je ne crois pas que ce cas soit testé quelque part...
...n/java/fr/gouv/vitamui/cas/authentication/DelegatedSurrogateAuthenticationPostProcessor.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/authentication/UserPrincipalResolver.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/authentication/UserPrincipalResolver.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/config/WebflowConfig.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/config/WebflowConfig.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/webflow/actions/CustomerSelectedAction.java
Show resolved
Hide resolved
...ver/src/main/java/fr/gouv/vitamui/cas/config/CustomSurrogateInitialAuthenticationAction.java
Show resolved
Hide resolved
...ver/src/main/java/fr/gouv/vitamui/cas/config/CustomSurrogateInitialAuthenticationAction.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/model/CustomerModel.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/pm/IamPasswordManagementService.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/pm/IamPasswordManagementService.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/pm/ResetPasswordController.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/webflow/actions/CustomerSelectedAction.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/webflow/actions/DispatcherAction.java
Show resolved
Hide resolved
...-server/src/main/java/fr/gouv/vitamui/cas/webflow/actions/GeneralTerminateSessionAction.java
Show resolved
Hide resolved
...c/main/java/fr/gouv/vitamui/cas/webflow/actions/I18NSendPasswordResetInstructionsAction.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/webflow/actions/ListCustomersAction.java
Show resolved
Hide resolved
cas/cas-server/src/main/java/fr/gouv/vitamui/cas/webflow/actions/ListCustomersAction.java
Show resolved
Hide resolved
numériques (0-9) | ||
</li> | ||
<li> | ||
caractères spéciaux (!"#$%&£'()*+,-./:;<=>?@[]^_`{|}~) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
L'objectif de cette PR est de faire évoluer Vitamui pour q'un même domaine mail puisse pouvoir être intégré dans deux organisations différentes afin de pouvoir accéder aux archives des organisations.
Cette évolution concerne les parties suivantes :
Type de changement:
Nouveau Code
Correction
Tests:
manuel
environnement
TU
Contributeur
VAS (Vitam Accessible en Service)