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

Adding master password decrypting #4753

Merged
merged 13 commits into from
Dec 8, 2024

Conversation

svaningelgem
Copy link
Contributor

@svaningelgem svaningelgem commented Dec 6, 2024

What's changed?

Adding the ability for openrewrite to read encrypted passwords. (maven-feature as described here:
https://maven.apache.org/guides/mini/guide-encryption.html

What's your motivation?

We can't use this library on our internal Nexus server as our passwords are encrypted.

Anything in particular you'd like reviewers to focus on?

For now this is a draft as I don't know what to add/change. Or maybe you can pick it up and whip it into shape? The heavy lifting for the decryption is already done after all ;-).

Have you considered any alternatives or workarounds?

No alternatives exist.

Any additional context

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • rewrite-maven/src/main/java/org/openrewrite/maven/MavenSettings.java
    • lines 27-27

@svaningelgem
Copy link
Contributor Author

@timtebeek : could you list all the changes that are necessary?

@timtebeek
Copy link
Contributor

Looking good already @svaningelgem ! Could you remove the ChangeParentPom2Test and replace it with a test that has both a security-settings.xml and settings.xml, based on the examples here, and verifies that the decrypted values match their actual values after calling maybeDecryptPasswords?

@svaningelgem
Copy link
Contributor Author

Thanks for the assist. I'll handle the hint you gave me

@svaningelgem
Copy link
Contributor Author

Hello @timtebeek , I added the tests now. Could you have a look if they're decent enough and/or where I would need to make changes?

Thanks

@timtebeek timtebeek marked this pull request as ready for review December 8, 2024 11:51
@svaningelgem
Copy link
Contributor Author

👍 Thanks @timtebeek for the assist ! :-)

Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the hard work here @svaningelgem ! I've only added some small changes to move things around and support relocation; apart from that very glad to see you tackled the decryption & tests. Hope this helps you all there!

@svaningelgem
Copy link
Contributor Author

Yes, I will try to test it out at work tomorrow. I hope this solves the issue.

@timtebeek timtebeek merged commit 16df13a into openrewrite:main Dec 8, 2024
2 checks passed
@svaningelgem svaningelgem deleted the security-settings-addition branch December 9, 2024 06:31
@sruffatti
Copy link

@svaningelgem This change is not working for me.

my master password is in a file called settings-security.xml in my .m2 directory

It looks like this

<settingsSecurity>
	<master>{my-encrypted-password}</master>
</settingsSecurity>

The exception is below.

java.lang.NegativeArraySizeException: -75
	at org.openrewrite.maven.MavenSecuritySettings.decrypt(MavenSecuritySettings.java:175)
	at org.openrewrite.maven.MavenSettings.lambda$maybeDecryptPasswords$4(MavenSettings.java:151)
	at org.openrewrite.internal.ListUtils.map(ListUtils.java:243)
	at org.openrewrite.internal.ListUtils.map(ListUtils.java:265)
	at org.openrewrite.maven.MavenSettings.maybeDecryptPasswords(MavenSettings.java:150)
	at org.openrewrite.maven.MavenSettings.readMavenSettingsFromDisk(MavenSettings.java:129)
	at com.org.engg.openrewrite.config.OrgRewriteTest.defaultExecutionContext(OrgRewriteTest.java:22)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:214)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:132)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:127)

@svaningelgem
Copy link
Contributor Author

Hi Sean,

What I would suggest is that you check out the main branch and debug with your settings file.
It is very possible that it won't work with every file as I skipped some stuff.
The current code is proven to work with my file and password, but likely yours is longer/shorter/more variation/... so it triggers a side condition.

Alternatively, you can send me your settings & settings security file (check my profile to see how to reach out). Watch out. This will expose your password to me (but I promise to delete it once I'm done with it -- and my memory is worse than a goldfish ;)).

Grtz,
Steven

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Authentication failure when using password encryption through Maven settings-security.xml
3 participants