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