You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/11/10 13:12:59 UTC
[GitHub] [james-project] ottoka opened a new pull request #741: Add a WebAdmin route for verifying a user password.
ottoka opened a new pull request #741:
URL: https://github.com/apache/james-project/pull/741
Extend the WebAdmin interface with a route to verify a username/password combination:
curl -XPOST http://ip:port/users/usernameToBeUsed \
-d '{"password":"passwordToBeVerified"}' \
-H "Content-Type: application/json"
The route reports 204 on success and 401 on failure. There is intentionally no distinction for non-existing users, to prevent a username oracle attack through this route.
Adding such a feature is useful for integrating James with 3rd party services, e.g. a web admin GUI.
T-Shirt size M.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ottoka commented on pull request #741: Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
ottoka commented on pull request #741:
URL: https://github.com/apache/james-project/pull/741#issuecomment-965462835
Right, I accidentally based the previous PR on an outdated local branch. This should be fine now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a change in pull request #741: JAMES-3667 Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #741:
URL: https://github.com/apache/james-project/pull/741#discussion_r747360419
##########
File path: server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java
##########
@@ -776,6 +776,48 @@ void allowedFromHeadersShouldReturn404WhenUserIsInvalid() {
.body("message", is("Invalid arguments supplied in the user request"));
}
+ @Test
+ void verifyShouldReturnOkWhenUserPasswordMatches() {
+ with()
+ .body("{\"password\":\"password\"}")
+ .put(USERNAME_WITH_DOMAIN.asString());
+
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .post(USERNAME_WITH_DOMAIN.asString() + "/verify")
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
Review comment:
We have as a convention the habit of putting `when()` and `then()` methods one indent back compare to the rest, to have a better visibility on the steps when sending a request in our tests. Something like this:
```
given()
.body("{\"password\":\"password\"}")
.when()
.post(USERNAME_WITH_DOMAIN.asString() + "/verify")
.then()
.statusCode(HttpStatus.NO_CONTENT_204);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #741: Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #741:
URL: https://github.com/apache/james-project/pull/741#issuecomment-965160414
> Adding such a feature is useful for integrating James with 3rd party services, e.g. a web admin GUI.
I sadly did not react to this on the mailing list but a GUI for webadmin (be it admin or user oriented...) surely is an area of interest... Contributions of such GUI would surely make James a better project ;-) That surely is a big topic, and I know some people around here might be interested too (@tuanlc ?).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #741: Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #741:
URL: https://github.com/apache/james-project/pull/741#issuecomment-965138320
Hello Karsten.
Thanks for the PR, I will surely get a look.
Can we open a JIRA for this topic? Can we prefix the comit with the JIRA title? (`JAMES-XXXX Add a WebAdmin route for verifying a user password`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a change in pull request #741: JAMES-3667 Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #741:
URL: https://github.com/apache/james-project/pull/741#discussion_r747156460
##########
File path: server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc
##########
@@ -473,6 +473,23 @@ Response codes:
This also can be used to create a new user.
+
+=== Verifying a user password
+
+....
+curl -XPOST http://ip:port/users/usernameToBeUsed \
Review comment:
Should we have a new path for this?
IMO, `users/usernameToBeUsed` looks like a common path
Ex:
- `/users/usernameToBeUsed/authen-check`
- `/users/authen-check`
- ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #741: JAMES-3667 Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #741:
URL: https://github.com/apache/james-project/pull/741#issuecomment-966886003
(relaunched a build...)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ottoka commented on a change in pull request #741: JAMES-3667 Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
ottoka commented on a change in pull request #741:
URL: https://github.com/apache/james-project/pull/741#discussion_r747300756
##########
File path: server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc
##########
@@ -473,6 +473,23 @@ Response codes:
This also can be used to create a new user.
+
+=== Verifying a user password
+
+....
+curl -XPOST http://ip:port/users/usernameToBeUsed \
Review comment:
Good idea, it makes the intention more clear, and avoids unfortunate accidents if someone confuses POST and PUT. I will stick with "verify" as action verb though, as authentication is more associated with JWT generation.
curl -XPOST http://ip:port/users/usernameToBeUsed/verify ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa merged pull request #741: JAMES-3667 Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #741:
URL: https://github.com/apache/james-project/pull/741
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ottoka commented on a change in pull request #741: Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
ottoka commented on a change in pull request #741:
URL: https://github.com/apache/james-project/pull/741#discussion_r746589841
##########
File path: server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
##########
@@ -112,8 +117,29 @@ public void define(Service service) {
defineAllowedFromHeaders();
defineUserExist();
+
+ defineVerifyUsers();
}
+ @POST
+ @Path("/{username}")
+ @ApiOperation(value = "Verifying User Password")
+ @ApiImplicitParams({
+ @ApiImplicitParam(required = true, dataType = "string", name = "username", paramType = "path"),
+ @ApiImplicitParam(required = true, dataType = "org.apache.james.webadmin.dto.VerifyUserRequest", paramType = "body")
+ })
+ @ApiResponses(value = {
+ @ApiResponse(code = HttpStatus.NO_CONTENT_204, message = "Verification successful."),
+ @ApiResponse(code = HttpStatus.UNAUTHORIZED_401, message = "Verification failed."),
+ @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Invalid input user."),
+ @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500,
+ message = "Internal server error - Something went bad on the server side.")
+ })
Review comment:
I noticed that as well, currently working on updating this PR so it works with the current master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ottoka commented on pull request #741: Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
ottoka commented on pull request #741:
URL: https://github.com/apache/james-project/pull/741#issuecomment-965550594
https://issues.apache.org/jira/browse/JAMES-3667 is the associated Jira issue.
Last time they were linked somehow, but not his time. How do I do this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ottoka commented on a change in pull request #741: JAMES-3667 Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
ottoka commented on a change in pull request #741:
URL: https://github.com/apache/james-project/pull/741#discussion_r747300756
##########
File path: server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc
##########
@@ -473,6 +473,23 @@ Response codes:
This also can be used to create a new user.
+
+=== Verifying a user password
+
+....
+curl -XPOST http://ip:port/users/usernameToBeUsed \
Review comment:
Good idea, it makes the intention more clear, and avoids unfortunate accidents if someone confuses POST and PUT. I will stick with "verify" as action verb though, as authentication is more associated with JWT generation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ottoka edited a comment on pull request #741: JAMES-3667 Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
ottoka edited a comment on pull request #741:
URL: https://github.com/apache/james-project/pull/741#issuecomment-965550594
https://issues.apache.org/jira/browse/JAMES-3667 is the associated Jira issue.
I renamed the pull request and commit, so the bot can link them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ottoka commented on a change in pull request #741: JAMES-3667 Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
ottoka commented on a change in pull request #741:
URL: https://github.com/apache/james-project/pull/741#discussion_r747381281
##########
File path: server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java
##########
@@ -776,6 +776,48 @@ void allowedFromHeadersShouldReturn404WhenUserIsInvalid() {
.body("message", is("Invalid arguments supplied in the user request"));
}
+ @Test
+ void verifyShouldReturnOkWhenUserPasswordMatches() {
+ with()
+ .body("{\"password\":\"password\"}")
+ .put(USERNAME_WITH_DOMAIN.asString());
+
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .post(USERNAME_WITH_DOMAIN.asString() + "/verify")
+ .then()
+ .statusCode(HttpStatus.NO_CONTENT_204);
Review comment:
Oops, evil auto-formatting at work. I actually noticed this in other tests. Fixed in latest push.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #741: Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #741:
URL: https://github.com/apache/james-project/pull/741#discussion_r746587847
##########
File path: server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
##########
@@ -112,8 +117,29 @@ public void define(Service service) {
defineAllowedFromHeaders();
defineUserExist();
+
+ defineVerifyUsers();
}
+ @POST
+ @Path("/{username}")
+ @ApiOperation(value = "Verifying User Password")
+ @ApiImplicitParams({
+ @ApiImplicitParam(required = true, dataType = "string", name = "username", paramType = "path"),
+ @ApiImplicitParam(required = true, dataType = "org.apache.james.webadmin.dto.VerifyUserRequest", paramType = "body")
+ })
+ @ApiResponses(value = {
+ @ApiResponse(code = HttpStatus.NO_CONTENT_204, message = "Verification successful."),
+ @ApiResponse(code = HttpStatus.UNAUTHORIZED_401, message = "Verification failed."),
+ @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Invalid input user."),
+ @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500,
+ message = "Internal server error - Something went bad on the server side.")
+ })
Review comment:
On master we got rid of the Swagger documentation that nobody was maintaining.
You may wish to rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ottoka commented on a change in pull request #741: Add a WebAdmin route for verifying a user password.
Posted by GitBox <gi...@apache.org>.
ottoka commented on a change in pull request #741:
URL: https://github.com/apache/james-project/pull/741#discussion_r746798851
##########
File path: server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
##########
@@ -112,8 +117,29 @@ public void define(Service service) {
defineAllowedFromHeaders();
defineUserExist();
+
+ defineVerifyUsers();
}
+ @POST
+ @Path("/{username}")
+ @ApiOperation(value = "Verifying User Password")
+ @ApiImplicitParams({
+ @ApiImplicitParam(required = true, dataType = "string", name = "username", paramType = "path"),
+ @ApiImplicitParam(required = true, dataType = "org.apache.james.webadmin.dto.VerifyUserRequest", paramType = "body")
+ })
+ @ApiResponses(value = {
+ @ApiResponse(code = HttpStatus.NO_CONTENT_204, message = "Verification successful."),
+ @ApiResponse(code = HttpStatus.UNAUTHORIZED_401, message = "Verification failed."),
+ @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Invalid input user."),
+ @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500,
+ message = "Internal server error - Something went bad on the server side.")
+ })
Review comment:
Removed the Swagger documentation in the force push below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org