Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRELEASE-1103] Wrong file path used for master pw file #181

Merged
merged 1 commit into from Apr 20, 2023

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Apr 14, 2023

Master pw path: Using plexus default, that is NOT same as Maven default.
Fix: we already inject DefaultPlexusSecDispatcher (not the iface), and it has setter, so use it.


https://issues.apache.org/jira/browse/MRELEASE-1103

@cstamas cstamas self-assigned this Apr 14, 2023
@cstamas
Copy link
Member Author

cstamas commented Apr 14, 2023

Interesting: branch has 4 commits, but this PR shows only 3 of them?

@cstamas cstamas requested a review from michael-o April 14, 2023 21:13
@cstamas
Copy link
Member Author

cstamas commented Apr 15, 2023

@slawekjaranowski ?

@slawekjaranowski
Copy link
Member

slawekjaranowski commented Apr 15, 2023

There is next place where we copy code and have many implementations for the same things ...
Eg in MavenCli we do similar ...

We can think about dedicated service for decryption/encryption which can use more secure system keychains or external crypto devices - but it is another topic ... I'm not an expert on it ... 😄

@@ -63,6 +63,9 @@ private MavenCryptoException(String message, Throwable cause) {
public MavenCrypto(DefaultSecDispatcher secDispatcher, PlexusCipher plexusCipher) {
this.secDispatcher = secDispatcher;
this.plexusCipher = plexusCipher;

// Adjust the default path (def path != maven path)
this.secDispatcher.setConfigurationFile("~/.m2/settings-security.xml");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work? I'm not sure. Windows and even on unix system?
It would be better to use something such System.getProperty("user.home")+"/.m2/settings-security.xml"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, sec dispatcher resolves this for us.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto. I wish Java could do it as well :)
well maybe a comment as it is not very intuitive. Image when some archeologists will have a look at this code with the new Maven dependencies cleanup in few years

…(works with 2.5.3)

Wrong file path used for master password file. Using plexus default, that is NOT
same as Maven default.

This closes #181
@asfgit asfgit force-pushed the MRELEASE-1103-wrong-settings-security-path branch from c0b122a to 2480664 Compare April 20, 2023 07:48
@asfgit asfgit closed this in 2480664 Apr 20, 2023
@asfgit asfgit merged commit 2480664 into master Apr 20, 2023
4 of 8 checks passed
@michael-o michael-o deleted the MRELEASE-1103-wrong-settings-security-path branch April 20, 2023 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants