Load/save from/to Github Repo

[stevage]

This was tracked in #3, which was partially implemented with Gist support, but no repo saving/loading yet.

[orangemug]

I'm going to be working on this over the next week.

[orangemug]

Ok so I'm still working on this, there were a few other things that needed doing first. So a quick update as to where this is at.

The working branch is here orangemug/editor@fix/update-deps...orangemug:feature/github-v2

It's basically hacked together in order to prove out the concept, it seems to work great. Here's what works (try to ignore the global variables 😱)

1. Login with github

2. List all a users repositories
3. Load a style from a repository (currently tries to load style.json in the base of a repository)

4. Commit back to that repository

These commits were made through the Maputnik UI using that process https://github.com/orangemug-test/style-test/commits/master

On my local machine this is all running inside docker containers via https://github.com/orangemug/maputnik-services including the GitHub authentication via micro-github (see https://github.com/orangemug/maputnik-services/blob/master/docker-compose.yml#L27-L43).

I've also started on a flags section it's basically chrome://flags for Maputnik. This is so I can release it early to get feedback, although releasing it is a few days off yet (probably towards middle of next week)

Next up, find a place to host micro-github, I'm currently thinking https://zeit.co/pricing but if anyone has any better ideas feel free to share.

As always any feedback is welcome.

[stevage]

Sounds great. Couple of quick comments:

• Some users have a lot of repos (I have >100). Listing them all could be problematic.
• Sounds like you're not supporting the workflow of either creating a new repo, or saving to a repo that you didn't load from. Or sending a PR. You might want to spell that out to the user with a message at the appropriate time.

[orangemug]

@stevage thanks for the feedback. Basically my aim initially is to to get the core working so we can iterate. I'll have a think about the above comments and get back to you. Any UI suggestions are also welcome.

[gregwolanski]

It's more complicated than I anticipated. :)

I'll try to take a look at it tomorrow.

[gregwolanski]

OK, today I end on:

npm ERR! Linux 4.9.49-moby
npm ERR! argv "/usr/bin/nodejs" "/usr/bin/npm" "install" "-d" "--dev"
npm ERR! node v6.1.0
npm ERR! npm  v3.8.6
npm ERR! path /maputnik/node_modules/jsonlint
npm ERR! code EXDEV
npm ERR! errno -18
npm ERR! syscall rename

npm ERR! EXDEV: cross-device link not permitted, rename '/maputnik/node_modules/jsonlint' -> '/maputnik/node_modules/.jsonlint.DELETE'
npm ERR!
npm ERR! If you need help, you may report this error at:
npm ERR!     <https://github.com/npm/npm/issues>

npm ERR! Please include the following file with any support request:
npm ERR!     /maputnik/npm-debug.log
ERROR: Service 'editor' failed to build: The command '/bin/sh -c npm install -d --dev' returned a non-zero code: 238

I'll get back to it tomorrow.

[orangemug]

Hey @gregorywolanski I wouldn't bother testing this yet, as it's still a work in progress.

[oterral]

Hi @orangemug we would like to add this save/load via github thing . Do you thing starting with your branch orangemug/editor@fix/update-deps...orangemug:feature/github-v2 is a good starting point ? do you have hints to provide before I start?

[orangemug]

Hey @oterral glad for the help.

So there is a reason why this hasn't been worked on in a while (sorry for that). I've been fixing other parts of the codebase for a new UI to make GitHub integration make sense.

See #252 https://281-84182601-gh.circle-artifacts.com/0/tmp/circle-artifacts.YPaobNs/build/index.html#0.36/0/0 basically we also need somewhere to store git history in context to the UI and also context for what repository is open. I think that UI gives us those abilities.

There are also a few other decisions (features) in that branch

• UI for feature flags
• Router (can be removed)

Lets create a plan of action and then we can divide up into smaller tickets, I'm happy to help out.

So things to do

• Export as Gist - same UI as before but we need a place for GitHub authentication
• Design the UI, read through Toolbar placement alteration #252 and let me know your thoughts. The filename in the top bar gives the user context on what is currently open. Git history would become a 'panel' in that new UI although that can be added later.

I'm pretty busy for the next couple of days but will try to find the time to respond.

[oterral]

I have 2 weeks full time to work on this.

What we want is:
- the possibility to log in then add a github repo
- possibility to browse this repo to find a style file
- load this file
- save this file back in the repo.

We could potentially add multiple repositories.

Maybe all the github stuff could be another Icon in the left menu. If we do it correctly, any users could add in the same way different kind of repository , github is not only one.

Click on it, opens a first panel with the login page for github if not logged in or the list of repositories added if already logged in.

Click on repository, opens the 2nd panel wich allows to browse the current repository

Then when you find your style click it to load in the map.

Your thoughts?

[stevage]

Just a couple of comments from the sidelines:

• Consider how other version control platforms might be integrated in the future, especially Gitlab. So it should be "version control integration", not "Github integration"
• Consider how to make the UX make sense for people with no experience using Github, like most cartographers. Letting them create single-file repos would probably make sense.
• Your workflow above doesn't seem to have a way to save a style to a repo that it wasn't loaded from. People will very likely want to do that. (But if you do load from a repo, saving updates to there should be smooth and easy.)

[oterral]

Consider how other version control platforms might be integrated in the future, especially Gitlab. So it should be "version control integration", not "Github integration"

Yes It's important I think. we could also have a new button in the left menu which opens a first panel, with buttons "login with github", "login with gitlab" and others.

Your workflow above doesn't seem to have a way to save a style to a repo that it wasn't loaded from. People will very likely want to do that. (But if you do load from a repo, saving updates to there should be smooth and easy.)

Yes we want that too. But it's more something for the export menu, I guess. I need to think about it.

[orangemug]

So I have some thoughts. Basically my motivations here getting the right solution and not wasting anybodies time (with regards to development).

Maputnik doesn't currently have a server and I'd personally like to keep it that way. In this ticket I'm currently using https://github.com/mxstbr/micro-github via https://github.com/orangemug/maputnik-services which means Maputnik just talks to an auth server.

micro-github however is not very secure as far as I can tell. It sends back the auth token in the querystring, which means it will stay about in the browser history (can somebody confirm this).

I'd also be nervous about adding this into master unless we can get some other security concerns addressed. We basically have the same access as git push once authenticated which means if we have any security vulnerabilities those tokens would be exposed to the attacker. GitHub repos can disable force push but basically hardly anyone does, so the impact of an attack could be pretty big.

It's also worth noting that we could approach this a different way. @lukasmartinelli added filesystem access to the editor via maputnik/desktop we could extend filesystem access to add git support. This would also require less development because we'd instantly be supporting all git services. The downside is that we'd be moving towards a 'download' model for the editor.

Let me know your thoughts?

[oterral]

If I understand correctly, instead of using micro-github we could use an auth0 server or whatever auth server, am I right?

[oterral]

I had a look to maputnink-services with micro-github. It sends backs the access_token in the querystring and it stays in the history, but it's only temporary token. I don't think it's a hudge problem if you 'r using https. We could try to pass it as POST request or in the header instead of the query string.

Otherwise your branch works well. I can load/save without problem in github.

[orangemug]

First off @oterral thank you for looking at this issue. Sorry it's taken me some time to get back to you I've been super busy with the day job.

I am however going to suggest we hold off on this issue for the time being.

The reason being that I think we have some other serious usability issues to resolve. So currently styling for one of the major sources is easy just pick openmaptiles from the data sources and away you go. Styling one of your own sources is somewhat more difficult this is due to the ugliness that is https://. A common use case for using Maputnik seems to be

I have a file on my computer (shapefile/geojson) how do I style it?

Right now that's really a lot more difficult than it should be. The steps are basically upload GeoJSON to a server that supports HTTPS and CORs, which is not an ideal experience. Mostly I run Maputnik locally, because it's always running for development of the code base. Which means I'm not experiencing these issues.

Possible solutions to this problem are as follows

1. Allow http://
2. Make a desktop client
3. Some ugly hacks I have in mind with hidden http:// iframes and window.postMessage

Here's the catch for the branch in this ticket.

1. If we allow http:// we can't do auth because it would compromise security
2. If we make a desktop client interacting with the git repositories on disk directly would probably be a better approach because we'd get GitLab/BitBucket/GitHub support in one feature
3. I'll expand on the ugly hacks I have in mind later

I know some people will be disappointed. I am too, I really want Git support in Maputnik but it seems the right thing to hold off until the we have the other issues resolved.

[pathmapper]

Currently https://github.com/mapbox/geojson.io is being built again (:tada:):

https://github.com/tmcw/geojson.net

Maybe it's worth to take a look how geojson.net is going to implement GitHub integration, there is also micro-github involved:

tmcw/geojson.net#8
tmcw/geojson.net#18

Also geojson.net is using https:// and it is possible to load local files.

[oterral]

I understand all your concerns @orangemug but I don't think using maputnik as desktop software is an option for us (internal security issues.
I will continue to investigate.

Thx for the link @pathmapper indeed what geojson.io/net does is exactly what I need. I will have a look.

[oterral]

geojson.io uses https://github.com/prose/gatekeeper instead of micro-github. The big diffference I see is GateKeeper is only used to get the access_token safely. The SPA directly requests the authorize code and uses GateKeeper only to get the access_token which it receives back in the response. No redirect url with access_token in querystring.

[oterral]

Ithnik maputnik should do the same and asks the github token directly.

[orangemug]

@oterral yeah gatekeeper uses the correct approach. Temp token with the real token returned in the response 👍

Also geojson.net is using https:// and it is possible to load local files.

Yeah sorry @pathmapper I probably wasn't clear here. So the issue isn't loading styles but loading sources. Any sources served over http:// from a local computer that could be GeoJSON from a local dataset or some vector tiles server locally. Because Maputnik is served over https:// the cross origin policies in the browser will prevent the fetching of those resources.

So basically if you have a data source at http://localhost:3000/sample.geojson or http://localhost:3000/sample.mvt the browser will throw an error (because it's not https://)

I think it's a pretty common use case to have local data and services from other applications either locally (localhost) or on your local network. Also setting up https:// for a local resource is not easy because self signed certificates can't realistically be used without first getting your browser to accept them (lots of hassle)

[orangemug]

I understand all your concerns @orangemug but I don't think using maputnik as desktop software is an option for us

@oterral if you could drop a comment in #164 as to what you using Maputnik for, that'd be much appreciated.

[oterral]

I have finally a working version with github (browsing all the repos) and gatekeeper base on @orangemug work. See here:

https://github.com/oterral/maputnik-service

To make it work you need to put the app oauth client id in the editor too: https://github.com/oterral/editor/blob/5f91018cce2ba886d072ba2d364ddefe1b30f7e5/src/components/modals/GitHubSection.jsx#L17

[orangemug]

@oterral congrats on getting it working! I'm a bit busy for the next couple of days but will find time to take a proper look at the weekend.

Note: Also I think #164 (comment) has convinced me we should just add this to Maputnik master branch behind a flag for now if you're still interested.

[gregwolanski]

This issue appears in the answers to question “What annoys you about Maputnik?” in the survey (original spelling):

• no integration with Github to keep a project synced