-
Notifications
You must be signed in to change notification settings - Fork 129
Implement PKCS authentication #1158
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
Conversation
team/bundles/org.eclipse.core.pki/src/org/eclipse/core/pki/FingerprintX509.java
Fixed
Show resolved
Hide resolved
team/bundles/org.eclipse.core.pki/src/org/eclipse/core/pki/util/NormalizeAES256.java
Fixed
Show resolved
Hide resolved
team/bundles/org.eclipse.core.pki/src/org/eclipse/core/pki/util/SecureAES256.java
Fixed
Show resolved
Hide resolved
team/bundles/org.eclipse.core.pki/src/org/eclipse/core/pki/AuthenticationBase.java
Fixed
Show resolved
Hide resolved
Test Results 507 files - 129 507 suites - 129 31m 17s ⏱️ - 8m 43s For more details on these failures, see this check. Results for commit cb1b6ca. ± Comparison against base commit 31e1ef4. This pull request removes 379 tests.
♻️ This comment has been updated with latest results. |
I see you've been asking for reviews... This PR is not linked to the following issue so I had to hunt for this related issue: Also, you've typically been making chains of commits which makes it hard to review and it means this needs to be squashed in the end, which is even more work. Generally it would be better to have one commit, amend it each time, and force push it. The title of the PR is meaningless. In the end, I can find no overview or description of what this is all about. Why would I use it? For what would I use it? The PR contains unrelated changes: The code itself contains many TODOs, lots of e.printStackTrace(), System.err.println, and System.out.println. It seems all exceptions ignored, other than print statement. I've not see DebugLogger.printDebug before. What's that? Things like this don't follow Java naming conventions:
I would hope that nothing needs to find Java in some specific location/drive on my machine... There's effectively no documentation here. There are no tests either. To me this does not look finished and does not look ready for review. When it is ready, there needs to be documentation to describe the purpose and the impact. There need to be tests. In the end, someone with expertise in this area will be needed. It looks like days of work would be needed to review this properly making the motivation even more significant. |
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.
Many of these files have nothing to do with my submitted bundles and I dont understand why they are appearing as changed, because I did not change them, or rather knowingly change them.
@JavaJoeS in your IDE select any file you don't wanted to have changed, then select Replace With > Previous Revision after that, amend your commit and force push it, these should then be gone, before you add something to the index make sure you really want to submit that change or revert it otherwise. |
Yes, and use amend followed by a force push to keep the branch to a single commit. |
This reverts commit 5479253.
Attempting to squash down and merge |
To enable a PKCS12 context, use org.eclipse.core.pki.util.TemplateForPKIfile. Run it and it creates a template in your .eclipse directory. Edit as needed and then run eclipse. You should end up with a .pki file and your pki certificate passwd will get
encrypted and be available for subsequent eclipse instances. The context for SSL will be available for any eclipse bundle
that checks to see if it already exists prior to creating one, which is how it should work.
See this issue for details