-
Notifications
You must be signed in to change notification settings - Fork 244
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
Remove FileOnMasterKeyStroreSource and SECURITY-1322 migration #540
Remove FileOnMasterKeyStroreSource and SECURITY-1322 migration #540
Conversation
@@ -113,101 +113,6 @@ public void displayName() throws IOException { | |||
assertEquals(EXPECTED_DISPLAY_NAME, CredentialsNameProvider.name(new CertificateCredentialsImpl(null, "abc123", null, "password", storeSource))); | |||
} | |||
|
|||
@Test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the code these tests where testing has been deleted.
Whilst we could leave a test that shows the behaviour doesn't work, I don't see a point in that)
It has been 5 years since the security fix was introduced and the migration code occureed. Whilst an admin could have still (ab)used the CLI/REST to set a FileOnMaster they could not do this from the UI creating a disparity. This change removes the migration and the ability to use this via CLI/REST
5a1fd31
to
079a4cb
Compare
@jtnord Maybe you already know, but just in case, a few plugins use
|
Thanks. the compuware was written 4 years after the original deprecation so I have no inclination to fix that - will fix the other 2. |
@jtnord The PR title has a typo, I edited my comment once I realized, see https://github.com/search?type=code&q=+owner%3Ajenkinsci+FileOnMasterKeyStoreSource |
🤦 thanks! |
was deprecated 5 years ago and about to be removed. jenkinsci/credentials-plugin#540
Breaks PCT in |
BOM update just needs to be coordinated with jenkinsci/credentials-binding-plugin#310 and jenkinsci/pipeline-model-definition-plugin#723. |
@dwnusbaum No, jenkinsci/credentials-binding-plugin#310 is incomplete—that PR removed one usage of |
It has been 5 years since the security fix was introduced and the migration code occurred.
Whilst an admin could have still (ab)used the CLI/REST to set a FileOnMaster they could not do this from the UI creating a disparity.
This change removes the migration and the ability to use this via CLI/REST
As noted originally and as would be present in the Jenkins logs at
INFO
level:SECURITY-1322: Migrating FileOnMasterKeyStoreSource to UploadedKeyStoreSource. The containing item may need to be saved to complete the migration.
Users would be required to check that they have saved any jobs where migration has occurred before updating to ensure that the CertificateCredential is not lost.
Testing done
mvn verify
Submitter checklist