Bug 295050 - [api] request enhanced support of client-certificates
Summary: [api] request enhanced support of client-certificates
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P2 enhancement (vote)
Target Milestone: 3.6   Edit
Assignee: torsten CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on: 178122
Blocks:
  Show dependency tree
 
Reported: 2009-11-13 05:03 EST by torsten CLA
Modified: 2011-05-31 15:48 EDT (History)
1 user (show)

See Also:


Attachments
Patch for my change-request (31.15 KB, text/plain)
2009-11-13 05:04 EST, torsten CLA
no flags Details
mylyn/context/zip (4.81 KB, application/octet-stream)
2009-11-18 15:30 EST, Steffen Pingel CLA
no flags Details
Patch -- 2nd iteration. (37.62 KB, text/plain)
2009-11-20 08:41 EST, torsten CLA
steffen.pingel: iplog+
Details
updated patch (46.67 KB, patch)
2011-05-23 19:17 EDT, Steffen Pingel CLA
no flags Details | Diff
fix layout problem (1.46 KB, patch)
2011-05-23 20:08 EDT, Steffen Pingel CLA
no flags Details | Diff
settings dialog (48.73 KB, image/png)
2011-05-31 15:48 EDT, Steffen Pingel CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description torsten CLA 2009-11-13 05:03:43 EST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.4) Gecko/20091016 Firefox/3.5.4 (.NET CLR 3.5.30729)
Build Identifier: Trac 3.3.x 

Trac / Mylyn supports using client-certificates for https-connections. However, as all informations are passed on as Java-Variable it seems unhandy: A password may be visible, and you cannot have two different certificates for two Task-Repositories.

I have added this functionality and created a patch. Testing was limited (I had problems building a proper update-site), but done within the IDE and also with Eclipse 3.5 SR1 on Linux and MacOS.

It would be great, if you can review my patch and submit it into Mylyn. 

Reproducible: Always

Steps to Reproduce:
(feature request)
Comment 1 torsten CLA 2009-11-13 05:04:44 EST
Created attachment 152140 [details]
Patch for my change-request
Comment 2 Steffen Pingel CLA 2009-11-18 15:29:59 EST
Thanks for the patch. That's a great a feature addition and the implementation looks very good! 

I would like to change on minor detail. Instead of prompting the user for the client certificate details in case of a handshake failure I think it's better to propagate the exception. The handshake could fail for many reasons and it could be confusing to get a prompt for a client certificate in that case. It could make sense to validate the password though in case opening the keystore fails, right now an exception is logged but it would be nice if the user was informed instead and given an opportunity to enter the correct password. Does that make sense? 

If you are interested in making these changes and post a new iteration I can go ahead and submit the patch for IP review to the Eclipse Foundation (that's the standard process for patches exceeding 250 lines of code).
Comment 3 Steffen Pingel CLA 2009-11-18 15:30:35 EST
Created attachment 152510 [details]
mylyn/context/zip
Comment 4 torsten CLA 2009-11-19 04:13:14 EST
Thanks Steffen!

I will do the changes and post a new patch soon. I have looked again at the code and found the excpetion.

So I would not add a password-dialog to the SSL-Factory, but rather catch the exception and throw a new one. This can be catched by the caller and reacted accordingly. At least the repository-properties dialog needs a different handling (= update the user message at the top) compared to other actions (= pop up a password dialog).
Comment 5 torsten CLA 2009-11-20 08:41:35 EST
Created attachment 152701 [details]
Patch -- 2nd iteration.
Comment 6 Steffen Pingel CLA 2010-02-08 00:14:08 EST
Thanks Torsten. The patch looks very good. I have submitted a CQ to get the patch IP reviewed by the Eclipse Foundation so that we can hopefully apply the changes for the 3.4 release.
Comment 7 torsten CLA 2010-02-09 01:38:34 EST
Thanks Steffen for your effort ... I confirm that:

 1.  I am authored 100% of the material
 2.  I do have the rights to donate the content to Eclipse
 3.  I am contributing the content under the EPL

-- Torsten.
Comment 8 Steffen Pingel CLA 2010-02-12 20:15:29 EST
Thanks Torsten. The CQ got approved. I'm planning to get the patch applied within the next three weeks.
Comment 9 Steffen Pingel CLA 2010-03-05 01:15:16 EST
Unfortunately I'll have to postpone merging this patch until bug 304755 is resolved which blocks changes to the org.eclipse.mylyn.commons.net plug-in. I am definitely still planning to get this in for the 3.4 release.
Comment 10 Steffen Pingel CLA 2010-05-25 00:41:34 EDT
Sorry, this fell of the plate for 3.4 and we are now to close to the release to make this change. I aim to get this in early for 3.5.
Comment 11 Steffen Pingel CLA 2011-03-03 23:19:19 EST
Sorry, I didn't get around to applying the patch. I will try again for 3.6.
Comment 12 Steffen Pingel CLA 2011-05-19 10:54:45 EDT
Thanks for being so patient, Torsten, and thanks a lot for this great contribution! I have finally gotten around to working on the patch and will commit it to head shortly.

I'll mark this bug as resolved to include the contribution in the IP log. I'll track any remaining work ob bug 178122.
Comment 13 Steffen Pingel CLA 2011-05-23 19:17:03 EDT
Created attachment 196386 [details]
updated patch
Comment 14 Steffen Pingel CLA 2011-05-23 20:08:14 EDT
Created attachment 196390 [details]
fix layout problem
Comment 15 Steffen Pingel CLA 2011-05-31 15:48:41 EDT
Created attachment 197039 [details]
settings dialog