Remove FileOnMasterKeyStroreSource and SECURITY-1322 migration#540
Conversation
| assertEquals(EXPECTED_DISPLAY_NAME, CredentialsNameProvider.name(new CertificateCredentialsImpl(null, "abc123", null, "password", storeSource))); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
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
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 |
jtnord
changed the title
🤦 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 |
4 participants
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
INFOlevel: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 verifySubmitter checklist