Skip to content
This repository has been archived by the owner on Feb 3, 2022. It is now read-only.

2 new endpoints and a solved bug #29

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

2 new endpoints and a solved bug #29

wants to merge 4 commits into from

Conversation

bartverhaar
Copy link
Contributor

  1. Endpoint for OIV data
  2. Endpoint for authorized connection with geoserver
  3. Solved bug loosing password after chaning user

- solved bug password loss after changing a user
- add endpoint for authorized connection with geoserver
@bartverhaar bartverhaar requested a review from matthijsln May 6, 2020 08:30
Copy link
Member

@matthijsln matthijsln left a comment

Choose a reason for hiding this comment

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

This PR contains 3 changes, split these into 3 PR's.

I don't think adding a generic proxy for securing WMS'es like this is a good idea, especially not implemented as simple as this and that with two glaring security problems.

}

private static JSONArray objectsJson (Connection c) throws Exception {
List<Map<String,Object>> rows = DB.oivQr().query("select id " +
Copy link
Member

Choose a reason for hiding this comment

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

Connection c is not used

"from objecten.view_objectgegevens"
, new MapListHandler());

Set<Integer> layerIds = new HashSet(DB.oivQr().query("select object_id " +
Copy link
Member

Choose a reason for hiding this comment

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

Connection c is not used

Integer object_id = (Integer)row.get("id");
JSONObject object = rowToJson(row, true, true);
if(layerIds.contains(object_id)) {
object.put("heeft_verdiepingen", true);
Copy link
Member

Choose a reason for hiding this comment

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

Why not just do this in SQL?

}

private static JSONObject objectJson (Connection c, int id) throws Exception {
List<Map<String,Object>> rows = DB.oivQr().query("select basic.id " +
Copy link
Member

Choose a reason for hiding this comment

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

Connection c is not used


private static JSONObject objectJson (Connection c, int id) throws Exception {
List<Map<String,Object>> rows = DB.oivQr().query("select basic.id " +
" , '' as oms_nummer " +
Copy link
Member

Choose a reason for hiding this comment

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

This huge SQL string is ugly, move to SQL

}

out.flush();
out.close();
Copy link
Member

Choose a reason for hiding this comment

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

Should be in finally

@@ -0,0 +1,2 @@
ALTER TABLE organisation.wms ADD auth varchar NULL;
Copy link
Member

Choose a reason for hiding this comment

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

Security: this is leaked to the client in /viewer/api/organisation.json

out += "&" + paramName + "=";
for (int i = 0; i < paramValues.length; i++) {
String paramValue = paramValues[i];
out += paramValue;
Copy link
Member

Choose a reason for hiding this comment

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

Concatenating multiple parameter values is not how it works. This turns ?a=1&a=2&a=3 into a=123

out += "&" + paramName + "=";
for (int i = 0; i < paramValues.length; i++) {
String paramValue = paramValues[i];
out += paramValue;
Copy link
Member

Choose a reason for hiding this comment

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

This does not URL encode parameter values

if(path != null) {
String geoserverUrl = handleParams(getContext().getRequest()).replace("&path=" + path + "&url=", "");
URL url = new URL (geoserverUrl);
HttpURLConnection connection = (HttpURLConnection) url.openConnection();
Copy link
Member

Choose a reason for hiding this comment

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

Security: this is an open proxy: /viewer/api/geoserver/get?url=https://www.google.com/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants