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

SPARK-1989 create empty KeyStores #376

Merged
merged 3 commits into from Aug 19, 2017

Conversation

Alameyo
Copy link
Member

@Alameyo Alameyo commented Aug 19, 2017

New empty KeyStores are created each time when user open advanced panel and there are no existing KeyStores in user directory already.

@wrooot
Copy link
Contributor

wrooot commented Aug 19, 2017

What does it mean that user opens advanced panel? What if a user doesn't ever open it and just hits login on a fresh profile?

@Alameyo
Copy link
Member Author

Alameyo commented Aug 19, 2017

Then KeyStores are opened if exist, if not new empty KeyStores are created because TrustManagers/KeyManager need them but I don't save them. Anyway if there were no KeyStores then user shouldn't be able to connect until he add required certificates (and by that open advanced panel).
For next PR I am working on adding Java cacerts file so user will be able to connect with some default certificates.

@wrooot
Copy link
Contributor

wrooot commented Aug 19, 2017

The usual user-friendly model is that if an unknown/self-signed certificate is encountered, the app provides a dialog/popup with an immediate option to add this certificate to an exception list or do a one time login. A number of clients i have used do this, browsers do this. It can involve a few steps, but a user shouldn't be looking for a menu to add a certificate to exceptions. A list of default certificates won't solve this.

@wrooot wrooot requested a review from guusdk August 19, 2017 12:37
@Alameyo
Copy link
Member Author

Alameyo commented Aug 19, 2017

I agree, that is thing I should to add. I will work on that.

Copy link
Member

@guusdk guusdk left a comment

Choose a reason for hiding this comment

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

There's a lot of boilerplate code here. There are fragments of code, that are pretty much duplicates, but use different variables. Let's replace those with a method.

@guusdk
Copy link
Member

guusdk commented Aug 19, 2017

The popup is certainly desirable, but not part of the GSoC project. Let's focus on finishing that first. We'll create the pop-up directly afterwards.

@guusdk
Copy link
Member

guusdk commented Aug 19, 2017

Thanks :)

@guusdk guusdk merged commit 5380933 into igniterealtime:master Aug 19, 2017
@Alameyo Alameyo deleted the create_empty_KeyStores branch December 9, 2018 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants