You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2020/05/20 05:41:46 UTC
[GitHub] [maven] mthmulders commented on a change in pull request #345: MNG-5583 new feature PKI authentication to nexus repositories
mthmulders commented on a change in pull request #345:
URL: https://github.com/apache/maven/pull/345#discussion_r427750837
##########
File path: maven-artifact/src/main/java/org/apache/maven/artifact/repository/Authentication.java
##########
@@ -125,5 +126,301 @@ public void setPrivateKey( final String privateKey )
{
this.privateKey = privateKey;
}
+
+
+ /**
+ *
Review comment:
Could you remove the superfluous `*` lines?
##########
File path: maven-artifact/src/main/java/org/apache/maven/artifact/repository/Authentication.java
##########
@@ -21,6 +21,7 @@
/**
* Authentication
+ * <br>May 2020, MNG-5583 per endpoint PKI authentication
Review comment:
This comment can be moved to release notes or a changelog, it doesn't have to be part of the Javadoc.
##########
File path: maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
##########
@@ -197,6 +197,15 @@ else if ( request.isUpdateSnapshots() )
for ( Server server : decrypted.getServers() )
{
AuthenticationBuilder authBuilder = new AuthenticationBuilder();
+ //MNG-5583 per endpoint PKI authentication
+ authBuilder.addString( "getKeyAlias", server.getKeyAlias() );
Review comment:
Can't you re-use the constants you declared in one of the earlier pull requests here?
##########
File path: maven-settings/src/main/mdo/settings.mdo
##########
@@ -580,6 +580,92 @@
]]>
</description>
</field>
+
+
+ <field>
+ <name>trustStore</name>
+ <version>1.0.0+</version>
+ <description>
+ <![CDATA[
+ The path to the trust store. If not defined, the JRE's cacert store is used.
+ ]]>
+ </description>
+ <type>String</type>
+ </field>
+ <field>
+ <name>trustStorePassword</name>
+ <version>1.0.0+</version>
+ <description>
+ <![CDATA[
+ The password to the trust store.
+ ]]>
+ </description>
+ <type>String</type>
+ </field>
+ <field>
+ <name>trustStoreType</name>
+ <version>1.0.0+</version>
+ <description>
+ <![CDATA[
+ The type of trust store, default is JKS
+ ]]>
+ </description>
+ <type>String</type>
+ </field>
+ <field>
+ <name>keyStore</name>
+ <version>1.0.0+</version>
+ <description>
+ <![CDATA[
+ The path to the keystore used for authentication purposes, or null
+ ]]>
+ </description>
+ <type>String</type>
+ </field>
+
+ <field>
+ <name>keyStorePassword</name>
+ <version>1.0.0+</version>
+ <description>
+ <![CDATA[
+ Keystore password, can be null
+ ]]>
+ </description>
+ <type>String</type>
+ </field>
+ <field>
+ <name>keyAlias</name>
+ <version>1.0.0+</version>
+ <description>
+ <![CDATA[
+ Keystore if the key store has multiple key pairs, this can be used to explicitly
Review comment:
Could you remove the first word? I think it's superfluous.
##########
File path: maven-compat/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java
##########
@@ -93,7 +92,16 @@ public AuthenticationInfo getAuthenticationInfo( String id )
authInfo.setUserName( server.getUsername() );
authInfo.setPassword( server.getPassword() );
authInfo.setPrivateKey( server.getPrivateKey() );
+ //MNG-5583 per endpoint PKI authentication
Review comment:
This line is not needed here, we could put in the release notes or changelog.
##########
File path: maven-compat/src/main/java/org/apache/maven/repository/legacy/LegacyRepositorySystem.java
##########
@@ -602,6 +601,17 @@ private Authentication getAuthentication( RepositorySystemSession session, Artif
authCtx.get( AuthenticationContext.PASSWORD ) );
result.setPrivateKey( authCtx.get( AuthenticationContext.PRIVATE_KEY_PATH ) );
result.setPassphrase( authCtx.get( AuthenticationContext.PRIVATE_KEY_PASSPHRASE ) );
+ //MNG-5583 per endpoint PKI authentication
+ result.setKeyAlias( authCtx.get( "getKeyAlias" ) );
Review comment:
Can't you re-use the constants you declared in one of the earlier pull requests here?
##########
File path: maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
##########
@@ -197,6 +197,15 @@ else if ( request.isUpdateSnapshots() )
for ( Server server : decrypted.getServers() )
{
AuthenticationBuilder authBuilder = new AuthenticationBuilder();
+ //MNG-5583 per endpoint PKI authentication
Review comment:
This line is not needed here, we could put in the release notes or changelog.
##########
File path: maven-compat/src/main/java/org/apache/maven/repository/legacy/LegacyRepositorySystem.java
##########
@@ -602,6 +601,17 @@ private Authentication getAuthentication( RepositorySystemSession session, Artif
authCtx.get( AuthenticationContext.PASSWORD ) );
result.setPrivateKey( authCtx.get( AuthenticationContext.PRIVATE_KEY_PATH ) );
result.setPassphrase( authCtx.get( AuthenticationContext.PRIVATE_KEY_PASSPHRASE ) );
+ //MNG-5583 per endpoint PKI authentication
Review comment:
This line is not needed here, we could put in the release notes or changelog.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org