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

Save client connection #384

Merged
merged 35 commits into from
May 23, 2016
Merged

Save client connection #384

merged 35 commits into from
May 23, 2016

Conversation

rmader
Copy link
Contributor

@rmader rmader commented May 20, 2016

This is a big rewrite of the client connection.
The first commits clean up the code of the ClientConnection and make them work well (actually, the old code did not accept certificates for wrong names, as it was supposed to do)

The later ones implement different download options for peers and non peers as suggested in #351
The config has now three options for httpsclient.trustselfsignedcerts:

  • none: never trust self signed certificates
  • peers (default): allow self signed certificates for peers, not for other sources
  • all: always accept all certificates

This comes with a warning: by making peers the default, everyone behind a ssl-proxy has either to change the config setting or import the proxy cert to her system. I think it's fair to be a little bit more "secure by default"

Whenever downloading from peers now, we have to use downloadPeers instead of download now.
download assumes authentication, but can be flagged to be opportunistic
Pushes assume peers by default, but can explicitly flagged to not do it.
If this is confusing, I can make it more similar.

I hope I got all download calls to peers changed, but ther're some doGet calls I didn't rightly understand. They still use the non peer download.

@Orbiter can you have a look on this later?

Orbiter and others added 11 commits May 21, 2016 10:09
Redesigned the apps page to matrix style with on-hover display.
Updated the apps.md for adding a screenshot
Fixes #331 Added script to start.sh, to read the port value from the config file
of the Post object, which is now in the Query class. This caused that
all servlets had to be modified.
Now the class AbstractAPIHandler is able to handle user access rights.
That class gets an APIServiceLevel object when asked by
AbstractAPIHandler for getCustomServiceLevel. Therefore we can handle
all authorization tasks at a central place.
class. Started to migrate that with the "Hello" class. This shall be
done until all api.client and api.server Classes are moved to
appropriate org.loklak.api sub-packages
@Orbiter
Copy link
Member

Orbiter commented May 22, 2016

all authentication/authorization rules should be implemented in https://github.com/loklak/loklak_server/blob/master/src/org/loklak/server/AbstractAPIHandler.java which handles this for all servlets. They propagate their authorization rules using https://github.com/loklak/loklak_server/blob/master/src/org/loklak/server/APIHandler.java#L32

I know that this is still an implementation stub. Lets see how we can merge this. I’m afraid your code needs ann adoption to that strategy.

@rmader
Copy link
Contributor Author

rmader commented May 23, 2016

Ah ok. Right now we have code using the download fuctionality independently of AbstractAPIHandler at different places, even in DAO init.

Do I get it right that you

  1. want all downloads to be called through AbstractAPIHandler
  2. want AbstractAPIHandler do decide whether a url points to a peer and can therefor done over an unsave connection?

1 might be a bit of a hassle at the moment (or unnessecary for this pull request, it could be done in a follow up to make the all the loklak code comply with AbstractAPIHandler), but 2. seems quite important indeed.
Would 2. be enough to complete the pull in the moment? I'd like to avoid to make this pull request even bigger, as it was mainly intended to fix certificate checking and code cleanup @Orbiter

@rmader rmader merged commit 03817a6 into master May 23, 2016
@rmader rmader deleted the saveClientConnection branch May 25, 2016 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants