chibenwa commented on code in PR #2643:
URL: https://github.com/apache/james-project/pull/2643#discussion_r1959504038


##########
examples/custom-healthcheck/pom.xml:
##########
@@ -18,16 +18,16 @@
     under the License.
 -->
 <project xmlns="http://maven.apache.org/POM/4.0.0"; 
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; 
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
-    <parent>
-        <artifactId>examples</artifactId>
-        <groupId>org.apache.james.examples</groupId>
-        <version>3.9.0-SNAPSHOT</version>
-    </parent>
-    <modelVersion>4.0.0</modelVersion>
+  <modelVersion>4.0.0</modelVersion>

Review Comment:
   Please do not change pom ordering



##########
.github/workflows/publish.yml:
##########
@@ -0,0 +1,38 @@
+name: Publish

Review Comment:
   Apache James do not use github workflow, this file is not needed.



##########
mailbox/jpa/derby.log:
##########
@@ -0,0 +1,13 @@
+----------------------------------------------------------------
+Wed Aug 07 13:25:47 BDT 2024:

Review Comment:
   Please do not commit derby logs



##########
.gitignore:
##########
@@ -6,5 +6,8 @@ dockerfiles/run/**/keystore
 dockerfiles/run/**/glowroot
 .m2
 test-run.log
-build
 .mvn/.develocity
+build

Review Comment:
   We don't integrate local development environment in the gitignore. Do it in 
a user profile instead.



##########
examples/custom-healthcheck/pom.xml.bak:
##########
@@ -0,0 +1,61 @@
+<?xml version="1.0" encoding="UTF-8"?>

Review Comment:
   Please do not commit `.bak`



##########
.mvn/.gradle-enterprise/gradle-enterprise-workspace-id:
##########
@@ -0,0 +1 @@
+b3ujmit4lbgnhnywrot42dhdmi

Review Comment:
   IDEM unrelated



##########
server/apps/distributed-app/JMAP API 9th FEB 24.postman_collection.json:
##########
@@ -0,0 +1,1453 @@
+{

Review Comment:
   This looks unrelated



##########
server/apps/distributed-app/README.adoc:
##########
@@ -4,33 +4,132 @@
 
  * Java 21 SDK
 
-Firstly, create your own user network on Docker for the James environment:
-
-    $ docker network create --driver bridge james
-
 Third party compulsory dependencies:
 
  * Cassandra 4.0
  * OpenSearch 2.1.0
  * RabbitMQ-Management 3.8.18
  * Zenko Cloudserver or AWS S3
 
+== Features Added:
+
+* +++<del>+++Restricted email sending to outside the org. e.g; 
<user>.<org>@<domain> can only send emails to other users with the same <org> 
and <domain>+++</del>+++
+* Added Gmail style "+ addressing". If your email is <user>@<domain>, you can 
use <user>\+<abc>@<domain> anywhere and still get your emails.
+The server ignores any string following the "+" character until the "@" 
character.
+(more details: 
https://eit.ces.ncsu.edu/2023/02/gmail-plus-addressing-the-hidden-feature-that-can-help-you-get-more-out-of-your-inbox/)
+* +++<del>+++ Disabled all authentication methods except JWT Authentication 
(RS256). User creation no longer requires a password. +++</del>+++
+
+* Generating a JWT key pair: 
https://github.com/apache/james-project/blob/master/server/apps/distributed-app/docs/modules/ROOT/pages/configure/jmap.adoc#generating-a-jwt-key-pair
+* Added a webadmin route for deleting an entire group. Example cURL command:
+-----
+$ curl --location --request DELETE 
'http://ip:port/address/groups/myGroup@mydomain'
+-----
+* Added a webadmin route for deleting multiple groups. Example cURL command:
+-----
+$ curl --location --request DELETE 'http://ip:port/address/groups' --data-raw 
'[
+    "myGroup1@mydomain",
+    "myGroup2@mydomain"
+]'
+-----
+* Added a webadmin route for creating an empty group. Example cURL command:
+-----
+$ curl --location --request POST 
'http://ip:port/address/groups/myGroup@mydomain'
+-----
+* Added a webadmin route for adding multiple groups with members. Example cURL 
command:
+-----
+$ curl --location --request GET 'http://ip:port/address/groups/add-groups' \
+--data-raw '[
+  {
+    "groupAddr": "group1@mydomain",
+    "memberAddrs": [
+      "member1@mydomain"
+    ]
+  },
+  {
+    "groupAddr": "group2@mydomain",
+    "memberAddrs": [
+      "member2@mydomain",
+      "member3@mydomain"
+    ]
+  }
+]'
+-----
+Example response body:
+-----
+[
+    {
+        "address": "group1@mydomain",
+        "status": "success",
+        "reason": "",
+        "membersInfo": [
+            {
+                "address": "member1@mydomain",
+                "status": "success",
+                "reason": ""
+            }
+        ]
+    },
+    {
+        "address": "group2@mydomain",
+        "status": "success",
+        "reason": "",
+        "membersInfo": [
+            {
+                "address": "member2@mydomain",
+                "status": "success",
+                "reason": ""
+            },
+            {
+                "address": "member3@mydomain",
+                "status": "success",
+                "reason": ""
+            }
+        ]
+    }
+]
+-----
+* Added a webadmin route for checking groups' existence. Example cURL command:
+-----
+$ curl --location --request GET 'http://ip:port/address/groups/isExist' \
+--data-raw '[
+    "aksda.org@mydomain",
+    "group1.org@mydomain",
+]'
+-----
+Example response body:
+-----
+[
+    {
+        "address": "aksda.org@mydomain",
+        "status": "DoesNotExists",
+        "reason": ""
+    },
+    {
+        "address": "group1.org@mydomain",
+        "status": "Exists",
+        "reason": ""
+    },
+]
+-----
+
+
 [source]
-----
-$ docker run -d --network james -p 9042:9042 --name=cassandra cassandra:4.1.5
-$ docker run -d --network james -p 9200:9200 --name=opensearch --env 
'discovery.type=single-node' --env 'DISABLE_SECURITY_PLUGIN=true' --env 
'DISABLE_INSTALL_DEMO_CONFIG=true' opensearchproject/opensearch:2.14.0
-$ docker run -d --network james -p 5672:5672 -p 15672:15672 --name=rabbitmq 
rabbitmq:3.13.3-management
-$ docker run -d --network james -p 8000:8000 --env 
'REMOTE_MANAGEMENT_DISABLE=1' --env 'SCALITY_ACCESS_KEY_ID=accessKey1' --env 
'SCALITY_SECRET_ACCESS_KEY=secretKey1' --name=s3 
ghcr.io/scality/cloudserver:8cbe2c066b3505b26d339dc67315d1041b8c7f3a
-----
 
-== Docker distribution
+== Running the server method 1

Review Comment:
   method 1 and method 2 => Running the server with docker compose / with java



##########
server/apps/distributed-app/README.adoc:
##########
@@ -4,33 +4,132 @@
 
  * Java 21 SDK
 
-Firstly, create your own user network on Docker for the James environment:
-
-    $ docker network create --driver bridge james
-
 Third party compulsory dependencies:
 
  * Cassandra 4.0
  * OpenSearch 2.1.0
  * RabbitMQ-Management 3.8.18
  * Zenko Cloudserver or AWS S3
 
+== Features Added:
+
+* +++<del>+++Restricted email sending to outside the org. e.g; 
<user>.<org>@<domain> can only send emails to other users with the same <org> 
and <domain>+++</del>+++
+* Added Gmail style "+ addressing". If your email is <user>@<domain>, you can 
use <user>\+<abc>@<domain> anywhere and still get your emails.
+The server ignores any string following the "+" character until the "@" 
character.
+(more details: 
https://eit.ces.ncsu.edu/2023/02/gmail-plus-addressing-the-hidden-feature-that-can-help-you-get-more-out-of-your-inbox/)
+* +++<del>+++ Disabled all authentication methods except JWT Authentication 
(RS256). User creation no longer requires a password. +++</del>+++
+
+* Generating a JWT key pair: 
https://github.com/apache/james-project/blob/master/server/apps/distributed-app/docs/modules/ROOT/pages/configure/jmap.adoc#generating-a-jwt-key-pair
+* Added a webadmin route for deleting an entire group. Example cURL command:
+-----
+$ curl --location --request DELETE 
'http://ip:port/address/groups/myGroup@mydomain'
+-----
+* Added a webadmin route for deleting multiple groups. Example cURL command:
+-----
+$ curl --location --request DELETE 'http://ip:port/address/groups' --data-raw 
'[
+    "myGroup1@mydomain",
+    "myGroup2@mydomain"
+]'
+-----
+* Added a webadmin route for creating an empty group. Example cURL command:
+-----
+$ curl --location --request POST 
'http://ip:port/address/groups/myGroup@mydomain'
+-----
+* Added a webadmin route for adding multiple groups with members. Example cURL 
command:
+-----
+$ curl --location --request GET 'http://ip:port/address/groups/add-groups' \
+--data-raw '[
+  {
+    "groupAddr": "group1@mydomain",

Review Comment:
   Webadmin enhancements are not related to JMAP



##########
server/apps/distributed-app/docker-compose.yml:
##########
@@ -1,74 +1,35 @@
 version: '3'
-
 services:
-
-  james:

Review Comment:
   Please revert this it breaks the default docker compose



##########
server/apps/distributed-app/README.adoc:
##########
@@ -4,33 +4,132 @@
 
  * Java 21 SDK
 
-Firstly, create your own user network on Docker for the James environment:
-
-    $ docker network create --driver bridge james
-
 Third party compulsory dependencies:
 
  * Cassandra 4.0
  * OpenSearch 2.1.0
  * RabbitMQ-Management 3.8.18
  * Zenko Cloudserver or AWS S3
 
+== Features Added:
+
+* +++<del>+++Restricted email sending to outside the org. e.g; 
<user>.<org>@<domain> can only send emails to other users with the same <org> 
and <domain>+++</del>+++
+* Added Gmail style "+ addressing". If your email is <user>@<domain>, you can 
use <user>\+<abc>@<domain> anywhere and still get your emails.
+The server ignores any string following the "+" character until the "@" 
character.
+(more details: 
https://eit.ces.ncsu.edu/2023/02/gmail-plus-addressing-the-hidden-feature-that-can-help-you-get-more-out-of-your-inbox/)
+* +++<del>+++ Disabled all authentication methods except JWT Authentication 
(RS256). User creation no longer requires a password. +++</del>+++
+
+* Generating a JWT key pair: 
https://github.com/apache/james-project/blob/master/server/apps/distributed-app/docs/modules/ROOT/pages/configure/jmap.adoc#generating-a-jwt-key-pair
+* Added a webadmin route for deleting an entire group. Example cURL command:
+-----
+$ curl --location --request DELETE 
'http://ip:port/address/groups/myGroup@mydomain'
+-----
+* Added a webadmin route for deleting multiple groups. Example cURL command:
+-----
+$ curl --location --request DELETE 'http://ip:port/address/groups' --data-raw 
'[
+    "myGroup1@mydomain",
+    "myGroup2@mydomain"
+]'
+-----
+* Added a webadmin route for creating an empty group. Example cURL command:
+-----
+$ curl --location --request POST 
'http://ip:port/address/groups/myGroup@mydomain'
+-----
+* Added a webadmin route for adding multiple groups with members. Example cURL 
command:
+-----
+$ curl --location --request GET 'http://ip:port/address/groups/add-groups' \
+--data-raw '[
+  {
+    "groupAddr": "group1@mydomain",
+    "memberAddrs": [
+      "member1@mydomain"
+    ]
+  },
+  {
+    "groupAddr": "group2@mydomain",
+    "memberAddrs": [
+      "member2@mydomain",
+      "member3@mydomain"
+    ]
+  }
+]'
+-----
+Example response body:
+-----
+[
+    {
+        "address": "group1@mydomain",
+        "status": "success",
+        "reason": "",
+        "membersInfo": [
+            {
+                "address": "member1@mydomain",
+                "status": "success",
+                "reason": ""
+            }
+        ]
+    },
+    {
+        "address": "group2@mydomain",
+        "status": "success",
+        "reason": "",
+        "membersInfo": [
+            {
+                "address": "member2@mydomain",
+                "status": "success",
+                "reason": ""
+            },
+            {
+                "address": "member3@mydomain",
+                "status": "success",
+                "reason": ""
+            }
+        ]
+    }
+]
+-----
+* Added a webadmin route for checking groups' existence. Example cURL command:
+-----
+$ curl --location --request GET 'http://ip:port/address/groups/isExist' \
+--data-raw '[
+    "aksda.org@mydomain",
+    "group1.org@mydomain",
+]'
+-----
+Example response body:
+-----
+[
+    {
+        "address": "aksda.org@mydomain",
+        "status": "DoesNotExists",
+        "reason": ""
+    },
+    {
+        "address": "group1.org@mydomain",
+        "status": "Exists",
+        "reason": ""
+    },
+]
+-----
+
+
 [source]
-----
-$ docker run -d --network james -p 9042:9042 --name=cassandra cassandra:4.1.5
-$ docker run -d --network james -p 9200:9200 --name=opensearch --env 
'discovery.type=single-node' --env 'DISABLE_SECURITY_PLUGIN=true' --env 
'DISABLE_INSTALL_DEMO_CONFIG=true' opensearchproject/opensearch:2.14.0
-$ docker run -d --network james -p 5672:5672 -p 15672:15672 --name=rabbitmq 
rabbitmq:3.13.3-management
-$ docker run -d --network james -p 8000:8000 --env 
'REMOTE_MANAGEMENT_DISABLE=1' --env 'SCALITY_ACCESS_KEY_ID=accessKey1' --env 
'SCALITY_SECRET_ACCESS_KEY=secretKey1' --name=s3 
ghcr.io/scality/cloudserver:8cbe2c066b3505b26d339dc67315d1041b8c7f3a
-----
 
-== Docker distribution
+== Running the server method 1
+-----
 
-To import the image locally:
+$ docker compose -f docker-composeOLD.yml up
 
-[source]
-----
-docker image load -i target/jib-image.tar
-----
+-----
+
+== Running the server method 2
+-----
+
+$ cd server/apps/distributed-app/
+
+$ mvn clean install -DskipTests
+ OR
+$ mvn com.github.ekryd.sortpom:sortpom-maven-plugin:sort -Dsort.keepBlankLines 
-Dsort.predefinedSortOrder=custom_1 -DskipTests clean install

Review Comment:
   Do not break checkstyles in the first place!



##########
examples/custom-mailets/src/main/java/org/apache/james/examples/custom/mailets/IsDelayedForMoreThan.java:
##########
@@ -52,10 +52,10 @@ public IsDelayedForMoreThan() {
     public Collection<MailAddress> match(Mail mail) throws MessagingException {
         Date sentDate = mail.getMessage().getSentDate();
 
-        if 
(clock.instant().isAfter(sentDate.toInstant().plusMillis(maxDelay.toMillis()))) 
{
+        //if 
(clock.instant().isAfter(sentDate.toInstant().plusMillis(maxDelay.toMillis()))) 
{
             return ImmutableList.copyOf(mail.getRecipients());
-        }
-        return ImmutableList.of();
+        //}
+        //return ImmutableList.of();

Review Comment:
   These changes are unrelated to JMAP



##########
server/apps/distributed-app/james postman collection. 10_09_2024.json:
##########
@@ -0,0 +1,2075 @@
+{

Review Comment:
   I'm not sure those auto generated files are meant to be committed.



##########
server/apps/distributed-app/src/main/java/mailets/IgnoreSuffixOfPlusSign.java:
##########
@@ -0,0 +1,56 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package mailets;
+
+import static org.apache.james.TrimSuffixOfPlusSign.trimSuffixOfPlusSign;
+
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.GenericMailet;
+
+import com.github.steveash.guavate.Guavate;
+
+
+public class IgnoreSuffixOfPlusSign extends GenericMailet {

Review Comment:
   locate this in a mailet package.
   Write java doc for it.
   Remove comment and system.out.
   
   Ah and now subaddressing is implemented on master, and trimming after + sign 
is the default behaviour
   
   So this mailet likely can be deleted.



##########
server/apps/distributed-app/rs256-4096-private.rsa:
##########
@@ -0,0 +1,52 @@
+-----BEGIN PRIVATE KEY-----

Review Comment:
   We intentionaly do not prepackage RSA crypto materials in order for people 
not to run that in prod, which is a backdoor.
   
   In place we should provide a mode for self-sign cry[pto generation. 
`server/apps/demo` does it.



##########
server/apps/distributed-app/pom.xml:
##########
@@ -338,10 +364,28 @@
             <artifactId>pdfbox</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>

Review Comment:
   missing scope test.
   
   And I fail at seing the added value of those maven changes



##########
server/apps/distributed-app/diffs.txt:
##########
@@ -0,0 +1,97 @@
+diff '--color=auto' conf/blob.properties docker-configuration/blob.properties

Review Comment:
   This file is irrelevant



##########
server/container/guice/cassandra/src/main/java/org/apache/james/modules/data/CassandraJmapModule.java:
##########
@@ -95,7 +95,7 @@ protected void configure() {
             .addBinding()
             .to(MessageFastViewProjectionHealthCheck.class);
 
-        bind(CassandraEmailQueryView.class).in(Scopes.SINGLETON);
+        
bind(CassandraEmailQueryView.class).in(Scopes.SINGLETON);https://github.com/apache/james-project/blob/master/server/container/guice/data-cassandra/src/main/java/org/apache/james/modules/data/CassandraUsersRepositoryModule.java

Review Comment:
   Does this even compiles?



##########
server/apps/distributed-app/start_docker_compose.sh:
##########
@@ -0,0 +1,38 @@
+#!/bin/bash

Review Comment:
   The entire point of docker compose is not to require a script to run it!



##########
server/apps/distributed-app/README.adoc:
##########
@@ -79,30 +185,35 @@ Note that you can create a domain via an environment 
variable. This domain will
       - DOMAIN=domain.tld
 ----
 
-== Running without docker
-
-To run james, you have to create a directory containing required configuration 
files.
-
-James requires the configuration to be in a subfolder of working directory 
that is called
-**conf**. A [sample 
directory](https://github.com/apache/james-project/tree/master/server/apps/distributed-app/sample-configuration)
-is provided with some default values you may need to replace, especially 
compulsory third party software endpoints.
-
-You will need to update its content to match your needs.
-
-Once everything is set up, you just have to run the jar with:
-
-[source]
-----
-$ java -Dworking.directory=. -Dlogback.configurationFile=conf/logback.xml 
-Djdk.tls.ephemeralDHKeySize=2048 -jar james-server-distributed-app.jar 
--generate-keystore
-----
-
-In the case of quick start James without manually creating a keystore (e.g. 
for development), just input the command argument `--generate-keystore` when 
running,
-James will auto-generate keystore file with the default setting that is 
declared in `jmap.properties` (tls.keystoreURL, tls.secret)
-
 [source]
-----
-$ java -Dworking.directory=. -Dlogback.configurationFile=conf/logback.xml 
-Djdk.tls.ephemeralDHKeySize=2048 -jar james-server-distributed-app.jar 
--generate-keystore
-----
-
-Note that binding ports below 1024 requires administrative rights.
 
+== Send emails using telnet
+
+```
+telnet 127.0.0.1 25
+EHLO spammer.com
+MAIL FROM: <sen...@spammer.com>
+RCPT TO: <recipi...@domain.tld>
+DATA
+Subject: This mail should be blocked
+
+Is it?
+.
+quit
+```
+
+```
+telnet 127.0.0.1 25
+EHLO spammer.com
+MAIL FROM: <anot...@spammer.com>
+RCPT TO: <recipi...@domain.tld>
+DATA
+Subject: This mail should be received
+
+Is it?
+.
+quit
+```

Review Comment:
   IMO that's irrelevant in a README



##########
server/apps/postgres-app/sample-configuration/smtpserver.xml:
##########
@@ -51,7 +51,7 @@
             <plainAuthEnabled>true</plainAuthEnabled>
         </auth>
         <authorizedAddresses>127.0.0.0/8</authorizedAddresses>
-        <verifyIdentity>true</verifyIdentity>
+        <verifyIdentity>false</verifyIdentity>

Review Comment:
   No please don't 
   Defaults are meant to keep people safe.
   Please do those changes on your own configuration / use a volume for 
overrides.



##########
server/apps/distributed-app/pom.xml:
##########
@@ -410,9 +455,9 @@
                         <image>eclipse-temurin:21-jre-jammy</image>
                     </from>
                     <to>
-                        <image>apache/james</image>
+                        <image>ghcr.io/appscode/inbox-server</image>

Review Comment:
   I'm unsure this is meant to be commited to an Apache project.



##########
server/data/data-api/pom.xml:
##########
@@ -50,10 +50,18 @@
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-configuration2</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.james</groupId>
+            <artifactId>james-server-helpers</artifactId>

Review Comment:
   What makes it different from james-server-utils ? Or james-core ?



##########
server/container/guice/lucene/pom.xml:
##########
@@ -42,4 +42,12 @@
         </dependency>
     </dependencies>
 
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+            </plugin>
+        </plugins>
+    </build>

Review Comment:
   This do not look needed...



##########
server/pom.xml:
##########
@@ -132,6 +133,15 @@
     <properties>
         <productName>Apache-James Mail Server</productName>
     </properties>
+    <dependencies>
+        <!-- https://mvnrepository.com/artifact/org.json/json -->
+        <dependency>
+            <groupId>org.json</groupId>
+            <artifactId>json</artifactId>

Review Comment:
   Please no do not import another JSON libraries. Use jackson databinding 
instead, which is widely used by the project.



##########
server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java:
##########
@@ -212,6 +212,14 @@ private String userExist(Request request, Response 
response) throws UsersReposit
 
     private HaltException upsertUser(Request request, Response response) 
throws Exception {
         Username username = extractUsername(request);
+        if (dummyUser.equals(username.asString())) {
+            LOGGER.info("Invalid username");
+            throw ErrorResponder.builder()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .type(ErrorType.INVALID_ARGUMENT)
+                    .message("Username supplied is invalid")
+                    .haltError();
+        }

Review Comment:
   This loks irrelevant: testing logic making it in production code.
   
   Please revert.



##########
server/apps/distributed-app/conf/blob.properties:
##########
@@ -0,0 +1,128 @@
+# ============================================= BlobStore Implementation 
==================================
+# Read https://james.apache.org/server/config-blobstore.html for further 
details
+
+# Choose your BlobStore implementation
+# Mandatory, allowed values are: cassandra, s3
+# *WARNING*: JAMES-3591 Cassandra is not made to store large binary content, 
its use will be suboptimal compared to
+# alternatives (namely S3 compatible BlobStores backed by for instance S3, 
MinIO or Ozone)
+implementation=cassandra
+
+# ========================================= Deduplication 
========================================

Review Comment:
   We described in the readme that sample-conf folder needed to be renamed in 
/conf folder.
   
   This was intendeed.



##########
server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/GroupsRoutes.java:
##########
@@ -22,10 +22,14 @@
 import static org.apache.james.webadmin.Constants.SEPARATOR;
 import static spark.Spark.halt;
 
+import java.util.ArrayList;

Review Comment:
   Please open one separate pull requests with only the proposed changes to 
GroupsRoutes



##########
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/DownloadRoutes.scala:
##########
@@ -261,7 +261,7 @@ class DownloadRoutes 
@Inject()(@Named(InjectionKeys.RFC_8621) val authenticator:
   private val blobIdParam: String = "blobId"
   private val nameParam: String = "name"
   private val contentTypeParam: String = "type"
-  private val downloadUri = s"/download/{$accountIdParam}/{$blobIdParam}"
+  private val downloadUri = s"/jmap/download/{$accountIdParam}/{$blobIdParam}"

Review Comment:
   Use an API gatewway / a reverse proxy like NGINX to customize the mapping if 
needed



##########
server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UsernameChangeRoutes.java:
##########
@@ -74,7 +78,15 @@ public Route changeUsername() {
             Username oldUser = Username.of(request.params(OLD_USER_PARAM));
             Username newUser = Username.of(request.params(NEW_USER_PARAM));
 
-            Preconditions.checkArgument(request.queryParams(FORCE_PARAM) != 
null || usersRepository.contains(oldUser), "'oldUser' parameter should be an 
existing user");
+            if (dummyUser.equals(newUser.asString())) {

Review Comment:
   Idem



##########
server/apps/distributed-app/src/main/java/matchers/NotInBlackList.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package matchers;
+
+import static org.apache.james.TrimSuffixOfPlusSign.trimSuffixOfPlusSign;
+
+import java.util.Collection;
+
+import org.apache.james.core.Domain;
+import org.apache.james.core.MailAddress;
+import org.apache.james.core.MaybeSender;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.GenericMatcher;
+
+import com.github.steveash.guavate.Guavate;
+
+
+public class NotInBlackList extends GenericMatcher {

Review Comment:
   Please learn to use composite matchers, there is a `Not`  matcher allowing 
for negation



##########
server/apps/distributed-app/src/main/java/matchers/NotInBlackList.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package matchers;
+
+import static org.apache.james.TrimSuffixOfPlusSign.trimSuffixOfPlusSign;
+
+import java.util.Collection;
+
+import org.apache.james.core.Domain;
+import org.apache.james.core.MailAddress;
+import org.apache.james.core.MaybeSender;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.GenericMatcher;
+
+import com.github.steveash.guavate.Guavate;
+
+
+public class NotInBlackList extends GenericMatcher {
+    NotInBlackList() {
+
+    }
+
+    @Override
+    public Collection<MailAddress> match(Mail mail) {
+        return mail.getRecipients()
+            .stream()
+            .filter(recipient -> !isSenderBlackListed(mail.getMaybeSender(), 
recipient))
+            .collect(Guavate.toImmutableList());
+    }
+
+    private Boolean isSenderBlackListed(MaybeSender maybeSender, MailAddress 
recipient) {
+        Domain domain = recipient.getDomain();
+
+        //System.out.println("receiver: " + recipient.getLocalPart() + " " + 
domain.toString());
+        //System.out.println("sender: " + maybeSender.get().getLocalPart() + " 
" + maybeSender.get().getDomain().toString());
+
+        String senderLocalPart = 
trimSuffixOfPlusSign(recipient).getLocalPart();
+        String recipientLocalPart = 
trimSuffixOfPlusSign(maybeSender.get()).getLocalPart();
+
+        if (giveOrg(senderLocalPart).equals(giveOrg(recipientLocalPart)) && 
domain.toString().equals(maybeSender.get().getDomain().toString())) {
+            //System.out.println("valid email");
+            return Boolean.FALSE;
+        }
+        //System.out.printf("invalid email");
+        return Boolean.TRUE;
+    }
+
+    public String giveOrg(String localpart) {
+        int startIndex = 0;
+
+        for (int i = localpart.length() - 2; i >= 0; i--) {
+           if (localpart.charAt(i) == '.') {
+               startIndex = i + 1;
+               break;
+           }
+        }
+        return localpart.substring(startIndex, localpart.length());

Review Comment:
   THis hacky code do not look safe



##########
server/data/data-api/src/main/java/org/apache/james/user/api/UsersRepository.java:
##########
@@ -151,9 +156,9 @@ public interface UsersRepository {
      */
     default Username getUsername(MailAddress mailAddress) throws 
UsersRepositoryException {
         if (supportVirtualHosting()) {
-            return 
Username.of(mailAddress.stripDetails(LOCALPART_DETAIL_DELIMITER).asString());
+            return Username.of(trimSuffixOfPlusSign(mailAddress).asString());
         } else {
-            return 
Username.of(mailAddress.stripDetails(LOCALPART_DETAIL_DELIMITER).getLocalPart());
+            return 
Username.of(trimSuffixOfPlusSign(mailAddress).getLocalPart());
         }

Review Comment:
   IDEM master supports out of the box subadressing



##########
server/pom.xml:
##########
@@ -132,6 +133,15 @@
     <properties>
         <productName>Apache-James Mail Server</productName>
     </properties>
+    <dependencies>

Review Comment:
   Dependencie management is done in the base POM
   
   Dependencies are only pulled in concrete implementations.



##########
server/protocols/webadmin/webadmin-core/pom.xml:
##########
@@ -82,6 +82,11 @@
             <artifactId>testing-base</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.auth0</groupId>
+            <artifactId>java-jwt</artifactId>
+            <version>4.2.1</version> <!-- Check for the latest version -->
+        </dependency>
         <dependency>

Review Comment:
   We use another JWT library,



##########
server/helpers/pom.xml:
##########
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"; 
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; 
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.james</groupId>
+        <artifactId>james-project</artifactId>
+        <version>3.9.0-SNAPSHOT</version>
+        <relativePath>../../pom.xml</relativePath>
+    </parent>
+
+    <artifactId>james-server-helpers</artifactId>
+    <version>3.9.0-SNAPSHOT</version>
+    <packaging>jar</packaging>
+
+    <properties>
+        <maven.compiler.source>21</maven.compiler.source>
+        <maven.compiler.target>21</maven.compiler.target>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+    </properties>

Review Comment:
   We should inherit those from parent.



##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/ValidRcptHandler.java:
##########
@@ -64,12 +65,13 @@ public void setSupportsRecipientRewriteTable(boolean 
supportsRecipientRewriteTab
 
     @Override
     protected boolean isValidRecipient(SMTPSession session, MailAddress 
recipient) throws UsersRepositoryException, RecipientRewriteTableException {
-        Username username = users.getUsername(recipient);
+        MailAddress trimmedRecipient = 
TrimSuffixOfPlusSign.trimSuffixOfPlusSign(recipient);
+        Username trimmedUsername = users.getUsername(trimmedRecipient);
 
-        if (users.contains(username)) {
+        if (users.contains(trimmedUsername)) {
             return true;
         } else {
-            return supportsRecipientRewriteTable && isRedirected(recipient, 
username.asString());
+            return supportsRecipientRewriteTable && 
isRedirected(trimmedRecipient, trimmedUsername.asString());
         }

Review Comment:
   No longer needed with master support for subadressing.



##########
server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/authentication/JwtFilter.java:
##########
@@ -53,12 +57,75 @@ public void handle(Request request, Response response) 
throws Exception {
 
             checkHeaderPresent(bearer);
             String login = retrieveUser(bearer);
-            checkIsAdmin(bearer);
+
+            Optional<String> userType = 
jwtTokenVerifier.verifyAndExtractClaim(bearer.get(), "type", String.class);
+            if (userType.isEmpty()) {
+                halt(HttpStatus.UNAUTHORIZED_401, "Missing user type in 
payload");
+            }
+
+            switch (userType.get()) {
+                case "agent":
+                    Optional<LinkedHashMap> permissionObject = 
jwtTokenVerifier.verifyAndExtractClaim(bearer.get(), "permissions", 
LinkedHashMap.class);
+
+                    if (permissionObject.isEmpty()) {
+                        halt(HttpStatus.UNAUTHORIZED_401, "Permissions claim 
not found.");
+                    }
+
+                    LinkedHashMap<String, List<String>> permissionClaims = new 
LinkedHashMap<>();
+                    permissionObject.get().forEach((key, value) -> {
+                        if (!(key instanceof String)) {
+                            throw new IllegalArgumentException("Invalid key 
type: " + key);
+                        }
+                        if (!(value instanceof List<?>)) {
+                            throw new IllegalArgumentException("Invalid value 
type for key '" + key + "': " + value);
+                        }
+                        List<?> valueList = (List<?>)value;
+                        for (Object item : valueList) {
+                            if (!(item instanceof String)) {
+                                throw new IllegalArgumentException("Invalid 
value type for List value for key '" + key + "' " + item);
+                            }
+                        }
+                        permissionClaims.put((String) key, (List<String>) 
value);
+                    });
+
+                    verifyAgentAuthorization(permissionClaims, request);
+                    break;

Review Comment:
   Please open a dedicated pull request with that one feature: `fine grained 
access control for webadmin`



-- 
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-unsubscr...@james.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org


Reply via email to