You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "vttranlina (via GitHub)" <gi...@apache.org> on 2023/04/18 01:56:16 UTC
[GitHub] [james-project] vttranlina opened a new pull request, #1525: Support `start-dev` argument when running Memory James server
vttranlina opened a new pull request, #1525:
URL: https://github.com/apache/james-project/pull/1525
- James will auto-generate keystore file with the default setting
## Why?
- Currently, when running James server (JMAP Server), For security reasons you are required to generate your own keystore, which you can mount into the container via a volume. -> Sometimes in the case of development, this requirement is a bit verbose
- Easier for newbies in the first step
- Security is a low priority for memory server
## How
- Make James support `start-dev` argument when starting (now available only for the memory server)
- If keystore file is not found & `start-dev`, James will auto-generate keystore file with the default setting
--
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 #1525: Support `start-dev` argument when running Memory James server
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa merged PR #1525:
URL: https://github.com/apache/james-project/pull/1525
--
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 pull request #1525: Support `start-dev` argument when running Memory James server
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #1525:
URL: https://github.com/apache/james-project/pull/1525#issuecomment-1534087380
The `ConfigurationSanitizingPerformer` looks pretty good. (y) Good job override
--
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 diff in pull request #1525: Support `start-dev` argument when running Memory James server
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1525:
URL: https://github.com/apache/james-project/pull/1525#discussion_r1173467634
##########
server/apps/memory-app/README.md:
##########
@@ -32,6 +32,14 @@ keytool -genkey -alias james -keyalg RSA -keystore keystore
docker run -v $PWD/keystore:/root/conf/keystore docker run apache/james:cassandra-latest
```
+In the case of quick start James without manually creating a keystore (e.g. for development), just input the command argument `start-dev` when running,
+James will auto-generate keystore file with the default setting:
+
+```
+docker run apache/james:memory-latest start-dev
Review Comment:
```suggestion
docker run apache/james:memory-latest --generate-keystore
```
Is more idiomatic from command line, and likely more self explanatory.
##########
server/apps/memory-app/README.md:
##########
@@ -32,6 +32,14 @@ keytool -genkey -alias james -keyalg RSA -keystore keystore
docker run -v $PWD/keystore:/root/conf/keystore docker run apache/james:cassandra-latest
```
+In the case of quick start James without manually creating a keystore (e.g. for development), just input the command argument `start-dev` when running,
+James will auto-generate keystore file with the default setting:
+
+```
+docker run apache/james:memory-latest start-dev
Review Comment:
We should document first how to auto-generate the keystore, then explain how to set up your own...
##########
server/apps/memory-app/README.md:
##########
@@ -32,6 +32,14 @@ keytool -genkey -alias james -keyalg RSA -keystore keystore
docker run -v $PWD/keystore:/root/conf/keystore docker run apache/james:cassandra-latest
```
+In the case of quick start James without manually creating a keystore (e.g. for development), just input the command argument `start-dev` when running,
+James will auto-generate keystore file with the default setting:
+
+```
+docker run apache/james:memory-latest start-dev
Review Comment:
Please document `--generate-keystore` in all other applications.
--
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 #1525: Support `start-dev` argument when running Memory James server
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1525:
URL: https://github.com/apache/james-project/pull/1525#issuecomment-1526974601
Todo:
- [ ] Manually test all APPs
--
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 diff in pull request #1525: Support `start-dev` argument when running Memory James server
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1525:
URL: https://github.com/apache/james-project/pull/1525#discussion_r1178557145
##########
server/apps/memory-app/README.md:
##########
@@ -32,6 +32,15 @@ keytool -genkey -alias james -keyalg RSA -keystore keystore
docker run -v $PWD/keystore:/root/conf/keystore apache/james:memory-latest
```
+In the case of quick start James without manually creating a keystore (e.g. for development),
+James memory server will auto-generate keystore file with the default setting that is declared in `jmap.properties` (tls.keystoreURL, tls.secret)
+Hence just run docker command:
+
+```
+docker run apache/james:memory-latest
Review Comment:
Base on your suggestion: "We can make start-dev the default for memory"
--
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 pull request #1525: Support `start-dev` argument when running Memory James server
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on PR #1525:
URL: https://github.com/apache/james-project/pull/1525#issuecomment-1514289166
https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1525/2/testReport/
Still some Guice injection issues to fix :)
Very good initiative otherwise, nice job ;)
--
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 diff in pull request #1525: Support `start-dev` argument when running Memory James server
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1525:
URL: https://github.com/apache/james-project/pull/1525#discussion_r1178548861
##########
server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/draft/JMAPConfigurationStartUpCheck.java:
##########
@@ -69,8 +105,71 @@ private CheckResult checkSecurityKey() {
}
}
+ private void loadSecurityKey() throws Exception {
+ try {
+ securityKeyLoader.load();
+ } catch (FileNotFoundException e) {
+ if (runArguments.contain(Argument.GENERATE_KEYSTORE)) {
+ LOGGER.warn("Can not load asymmetric key from configuration file" + e.getMessage());
+ LOGGER.warn("James will try the auto-generate an asymmetric key. It is just for development, should not use it on production");
+ generateKeystore();
+ } else {
+ throw e;
+ }
+ }
+ }
+
@Override
public String checkName() {
return CHECK_NAME;
}
+
+ private void generateKeystore() throws Exception {
Review Comment:
It is not the JMAP startUp checks that should generate the keystore...
##########
server/apps/memory-app/README.md:
##########
@@ -32,6 +32,15 @@ keytool -genkey -alias james -keyalg RSA -keystore keystore
docker run -v $PWD/keystore:/root/conf/keystore apache/james:memory-latest
```
+In the case of quick start James without manually creating a keystore (e.g. for development),
+James memory server will auto-generate keystore file with the default setting that is declared in `jmap.properties` (tls.keystoreURL, tls.secret)
+Hence just run docker command:
+
+```
+docker run apache/james:memory-latest
Review Comment:
Why not `--generate-keystore` ?
I advocate to do exactly the same that other servers...
--
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 #1525: Support `start-dev` argument when running Memory James server
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1525:
URL: https://github.com/apache/james-project/pull/1525#issuecomment-1524440765
Did you manually tested this for JPA?
--
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 diff in pull request #1525: Support `start-dev` argument when running Memory James server
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1525:
URL: https://github.com/apache/james-project/pull/1525#discussion_r1178574571
##########
server/apps/memory-app/README.md:
##########
@@ -32,6 +32,15 @@ keytool -genkey -alias james -keyalg RSA -keystore keystore
docker run -v $PWD/keystore:/root/conf/keystore apache/james:memory-latest
```
+In the case of quick start James without manually creating a keystore (e.g. for development),
+James memory server will auto-generate keystore file with the default setting that is declared in `jmap.properties` (tls.keystoreURL, tls.secret)
+Hence just run docker command:
+
+```
+docker run apache/james:memory-latest
Review Comment:
Well, now that I see this, I really wonder why we do anything special for memory ;-)
--
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 pull request #1525: Support `start-dev` argument when running Memory James server
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #1525:
URL: https://github.com/apache/james-project/pull/1525#issuecomment-1524304052
> We can make start-dev the default for memory, demo and not the others...
For demo, It already bash script to generate a private key., private.scr. So, we don't need generate keystore
--
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 pull request #1525: Support `start-dev` argument when running Memory James server
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #1525:
URL: https://github.com/apache/james-project/pull/1525#issuecomment-1524304762
Conflict, I will rebase to resolve that
--
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 diff in pull request #1525: Support `start-dev` argument when running Memory James server
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1525:
URL: https://github.com/apache/james-project/pull/1525#discussion_r1178558169
##########
server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/draft/JMAPConfigurationStartUpCheck.java:
##########
@@ -69,8 +105,71 @@ private CheckResult checkSecurityKey() {
}
}
+ private void loadSecurityKey() throws Exception {
+ try {
+ securityKeyLoader.load();
+ } catch (FileNotFoundException e) {
+ if (runArguments.contain(Argument.GENERATE_KEYSTORE)) {
+ LOGGER.warn("Can not load asymmetric key from configuration file" + e.getMessage());
+ LOGGER.warn("James will try the auto-generate an asymmetric key. It is just for development, should not use it on production");
+ generateKeystore();
+ } else {
+ throw e;
+ }
+ }
+ }
+
@Override
public String checkName() {
return CHECK_NAME;
}
+
+ private void generateKeystore() throws Exception {
Review Comment:
It is based on your suggestion "please move the key generation out of JMAP-draft. Suggestion: server/container/guice/common"
Can you give more detail?
I tried to move the SecurityKeyload to `server/container/guice/common`, but see it is a bit not correct, So I only move the generateKeystore method
--
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 diff in pull request #1525: Support `start-dev` argument when running Memory James server
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1525:
URL: https://github.com/apache/james-project/pull/1525#discussion_r1169413379
##########
server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/crypto/SecurityKeyLoader.java:
##########
@@ -45,25 +67,48 @@
import nl.altindag.ssl.util.PemUtils;
public class SecurityKeyLoader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(SecurityKeyLoader.class);
private static final String ALIAS = "james";
private final FileSystem fileSystem;
private final JMAPDraftConfiguration jmapDraftConfiguration;
+ private final RunArguments runArguments;
+
@VisibleForTesting
@Inject
- SecurityKeyLoader(FileSystem fileSystem, JMAPDraftConfiguration jmapDraftConfiguration) {
+ SecurityKeyLoader(FileSystem fileSystem,
+ JMAPDraftConfiguration jmapDraftConfiguration,
+ RunArguments runArguments) {
+ this.fileSystem = fileSystem;
+ this.jmapDraftConfiguration = jmapDraftConfiguration;
+ this.runArguments = runArguments;
+ }
+
+ SecurityKeyLoader(FileSystem fileSystem,
+ JMAPDraftConfiguration jmapDraftConfiguration) {
this.fileSystem = fileSystem;
this.jmapDraftConfiguration = jmapDraftConfiguration;
+ this.runArguments = RunArguments.empty();
}
public AsymmetricKeys load() throws Exception {
Preconditions.checkState(jmapDraftConfiguration.isEnabled(), "JMAP is not enabled");
- if (jmapDraftConfiguration.getKeystore().isPresent()) {
- return loadFromKeystore();
+ try {
+ LOGGER.info("tung keystore" + jmapDraftConfiguration.getKeystore());
Review Comment:
Oups
--
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 #1525: Support `start-dev` argument when running Memory James server
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1525:
URL: https://github.com/apache/james-project/pull/1525#issuecomment-1513321730
https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1525/1/testReport/junit/org.apache.james/AuthenticatedCassandraJamesServerTest$AuthenticationFailureTest/startShouldFailOnBadPassword_GuiceJamesServer_/
org.apache.james.AuthenticatedCassandraJamesServerTest$AuthenticationFailureTest.startShouldFailOnBadPassword{GuiceJamesServer} (from org.apache.james.AuthenticatedCassandraJamesServerTest)
```
Expecting actual:
"com.google.inject.CreationException: Unable to create injector, see the following errors:
1) [Guice/MissingConstructor]: No injectable constructor for type RunArguments.
class RunArguments does not have a @Inject annotated constructor or a no-arg constructor.
Requested by:
1 : RunArguments.class(RunArguments.java:29)
at SecurityKeyLoader.<init>(SecurityKeyLoader.java:82)
\_ for 3rd parameter
at JMAPCommonModule.configure(JMAPCommonModule.java:73)
\_ installed by: Modules$OverrideModule -> Modules$OverrideModule -> Modules$OverrideModule -> Modules$OverrideModule -> Modules$CombinedModule -> Modules$CombinedModule -> Modules$CombinedModule -> Modules$CombinedModule -> Modules$CombinedModule -> Modules$CombinedModule -> Modules$CombinedModule -> Modules$CombinedModule -> JMAPServerModule -> JMAPModule -> JMAPCommonModule
Learn more:
https://github.com/google/guice/wiki/MISSING_CONSTRUCTOR
1 error
======================
Full classname legend:
======================
JMAPCommonModule: "org.apache.james.jmap.draft.JMAPCommonModule"
JMAPModule: "org.apache.james.jmap.draft.JMAPModule"
JMAPServerModule: "org.apache.james.modules.protocols.JMAPServerModule"
Modules$CombinedModule: "com.google.inject.util.Modules$CombinedModule"
Modules$OverrideModule: "com.google.inject.util.Modules$OverrideModule"
RunArguments: "org.apache.james.RunArguments"
SecurityKeyLoader: "org.apache.james.jmap.draft.crypto.SecurityKeyLoader"
========================
End of classname legend:
========================
```
--
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 diff in pull request #1525: Support `start-dev` argument when running Memory James server
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1525:
URL: https://github.com/apache/james-project/pull/1525#discussion_r1178574698
##########
server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/draft/JMAPConfigurationStartUpCheck.java:
##########
@@ -69,8 +105,71 @@ private CheckResult checkSecurityKey() {
}
}
+ private void loadSecurityKey() throws Exception {
+ try {
+ securityKeyLoader.load();
+ } catch (FileNotFoundException e) {
+ if (runArguments.contain(Argument.GENERATE_KEYSTORE)) {
+ LOGGER.warn("Can not load asymmetric key from configuration file" + e.getMessage());
+ LOGGER.warn("James will try the auto-generate an asymmetric key. It is just for development, should not use it on production");
+ generateKeystore();
+ } else {
+ throw e;
+ }
+ }
+ }
+
@Override
public String checkName() {
return CHECK_NAME;
}
+
+ private void generateKeystore() throws Exception {
Review Comment:
I can try to get a look at it ^^
--
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