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

CREST Fitting Support #383

Closed
13 of 20 tasks
blitzmann opened this issue Oct 14, 2015 · 40 comments
Closed
13 of 20 tasks

CREST Fitting Support #383

blitzmann opened this issue Oct 14, 2015 · 40 comments
Labels
enhancement This is a feature request, or an idea to enhancement a current feature

Comments

@blitzmann
Copy link
Collaborator

blitzmann commented Oct 14, 2015

CCP FoxFour just release documentation on this today, see https://github.com/ccpgames/eveonline-third-party-documentation/commit/1ff6d7ac56c4b1c66b0e39155b502dfb7ec0336c

This issue will keep track of current progress.


  • Webserver support for gathering auth code. This will probably require rebuilding Windows and Mac skels
  • Fallback in case webserver is not viable due to user restrictions (unsure of mechanism here)
  • Database support for CREST Characters, which will hold the refresh token
  • Getting and displaying saved fittings from EVE server. This is done, and works in this way: Select the character and fetch characters. Tree displays. Select fitting, and the modules (and cargo/drones/whatever is attached to saved fitting) are loaded as pyfa Items and then passed into a Cargo wrapper (Cargo items display amount unlike Modules) and then displayed. This will work for the initial release, but will need to be iterated on. I want to make it look somewhat like the fitting view with the slots separated. This will take more work simply because that view assumes it's being attached to a fit and calculated with a character, which we don't want (or do we???). https://gyazo.com/6da86533ba59b09ce70ae873bccb583e
  • Importing saved fits
  • Exporting to CREST format - Done. This is somewhat of a pain as we need to know the invFlag for the module, which is different than what pyfa uses. I aim to make a simple converter between pyfa's internal position and the inf flag, but for now I've got a working thing.
  • Sending fit to server

Known issues: (as of 10/25/15)

  • Very little testing has been done with regards on how to handle errors from CCP (such as too many fittings)
  • Changing between the two modes does not work until pyfa is restarted -- menus need to know when the mode changes, as does the CREST service (so it may reload the settings).
  • Changing clientID and clientSecret and trying to access CREST with refresh token bound to the old settings hangs the program. The solution here would be to simply delete all saved characters
  • [backend] CREST Characters are not cached at the database level for some reason. I have a stopgap in place to cache them at the service level, but this is not ideal. This shouldn't affect end-users either way.
  • If internet connection is not available, CREST calls fail. Not sure of the best approach here. Maybe simply catch the exception when calling fetch / post / delete and display status message?
  • Need a message if response handling fails for some reason (bad state, incorrect info)
  • After adding characters via User-defined mode, menu items are disabled until restart. (the CREST menus are generally buggy and not working well, fix all these)

To-do's:

  • Support DELETE of fitting
  • force kill the server before starting again, just in case it's still running for whatever reason.
  • Possibly rework the response handling so that the "Done" message shown to the user only says that if everything actually worked. Maybe move the entirety of the response handle logic into AuthHandler
  • Work with pycrest's cache when adding / deleting fit so that we can at least keep an up-to-date copy on our end.
  • Merge the fit export with the clipboard export options
  • Possible search function for fit browser, and display currently selected fit and ship type above the module listing
@blitzmann blitzmann added the enhancement This is a feature request, or an idea to enhancement a current feature label Oct 14, 2015
@blitzmann
Copy link
Collaborator Author

OAUTH stuff is all new to me, so these are basically just ramblings

There are two options:

  • Authentication grant: this requires the client ID and client secret key, which means we would need a dedicated server that would perform the authorization to avoid revealing the secret key. pyfa would embed a web view showing a pyfa-hosted page on some server (or even maybe simply open the web browser to display), where the user would log in via SSO. The EVE servers would then validate and confirm with the user, and redirect to a callback on the pyfa server with an authorization code.

The upside to this is that we are allowed to use a refresh token for the user, which (if I understand correctly) we can store in pyfa and make requests without re-authenticating (which would get tiresome). This is also assuming that we would only need the dedicated server to get the auth code, and won't need to keep making requests back and forth afterwards -- the complexity of this grant type will increase drastically if we need to use the dedicated server for ALL traffic between pyfa and EVE.

http://tools.ietf.org/html/rfc6749#section-4.1

  • Implicit grant: does not require the client secret, and thus does not require us to use a dedicated server. The problem with this is that refresh tokens are not supported, so every 20 minutes the user would be presented with a dialog asking them to log into the SSO.

http://tools.ietf.org/html/rfc6749#section-4.2

Need to test: Register custom handler with the OS that which handle opening a pyfa://crestauth/?blah=blah URL and test functionality.

@narthollis
Copy link

You are correct that you can store a refresh token and use it to later get a fresh access token. (Assuming the refresh token hasn't expired - which you have no idea about until you make the request)

As for how to obtain the auth code and resolve that into an access/refresh token...

http://stackoverflow.com/questions/4419915/how-to-keep-the-oauth-consumer-secret-safe-and-how-to-react-when-its-compromis
https://developers.google.com/identity/protocols/OAuth2InstalledApp

It seams the general consensus is to just keep the secret in the app, probably adding it as part of the build process and telling source code users how to obtain their own secret. This would involve either having crest 'redirect' to a pyfa:// link (which if you have ever tried steam:// links is a bit funky) or use an embedded browser. (I don't think that CREST SSO currently has a display auth code in browser option)

Alternatively there is the proxy scenario where the client secret is kept on a 'secure' web service, but this could simply allow just about anyone to use that service to auth using your client secret anyway....

@blitzmann
Copy link
Collaborator Author

It seams the general consensus is to just keep the secret in the app, probably adding it as part of the build process

Unfortunately this is not an option. Having the secret shipped with the client immediately makes it insecure, especially in our situation and the language we use - it's very difficult to hide things in python -- even the EVE client can be decompiled back into it's source. So if we want to keep it hidden, then we shouldn't ship it

Alternatively there is the proxy scenario where the client secret is kept on a 'secure' web service, but this could simply allow just about anyone to use that service to auth using your client secret anyway....

If someone goes to a webpage specifically made for pyfa, signs into SSO, and is redirected to a webpage or application that is controlled by pyfa, no other application can really use that since that application will need to be redirected elsewhere? Hmm...

EDIT: But then if the Implicit grant doesn't even need a secret key, would it really be troublesome for another app to use the pyfa authentication? Ugh, this is confusing

@narthollis
Copy link

Proxy / Web Service:

Source Action Destination
Pyfa Get Token Web Service
Web Service Redirect (with client id) EVE SSO
EVE SSO Redirect (with auth code) Web Service
Web Service Get Token (with auth code, client id, and client secret) CREST
Web Service Return (with Token) Pyfa

The problem here is that for the web service to be even equal to storing the secret in the install, the application needs to prove in some way that it is allowed to request a token.

And so you are again in the position of having to store a secret in the application, just this time its a web service secret instead.

There is no good solution to the client secret problem. You can obfuscate the secret however you want, but even a heavily obfuscated secret in a compiled C/C++ application is going to be fairly easy to retrieve.

The opening page of the Google OAuth2 documentation states it quite clearly:

Installed apps are distributed to individual machines, and it is assumed that these apps cannot keep secrets.

It further states toward the end of the overview:

The client ID and client secret obtained from the Developers Console are embedded in the source code of your application. In this context, the client secret is obviously not treated as a secret.

The best thing that can be done is for the Pyfa client to have access to the smallest scope the makes sense for it to have, in this case simply fitting read/write.

OAuth was never really written with desktop applications in mind, so these are the problems we run into.

@blitzmann
Copy link
Collaborator Author

Yeah, was thinking about this last night. Even with a web proxy, pyfa would have to prove its pyfa somehow, whi h can be easily spoofed. Ill talk with foxfour to see what exactly the consequences might be for someone to obtain the client secret, cause it seems to me that it doesnt really matter (especially if we can already access the data without authenticatijg the client with implict grant)

@blitzmann
Copy link
Collaborator Author

I just got confirmation from FoxFour on TweetFleet Slack that it should be okay to ship with client secret as of right now. http://pastebin.com/TfWg1ywi

@regner
Copy link

regner commented Oct 16, 2015

I have asked a good friend who is a lot smarter than me to come and offer his feedback/suggestions. Posting mainly to subscribe to this issue and get an alert when he does respond.

@jimpurbrick
Copy link

OAuth 2 for native apps is fiddly. The easiest and clearest approach would be to launch a browser which opened a version of crestexplorerjs (https://github.com/jimpurbrick/crestexplorerjs) customized to display tokens to the user which can be copied in to pyfa.

The copying and pasting can be avoided using several mechanisms discussed here http://wiki.oauth.net/w/page/27249271/OAuth%202%20for%20Native%20Apps. Allowing the redirect URL to open a web server embedded in pyfa might be pretty slick and should work on all platforms, but you'd have to have the token copying fallback in case firewalls or permissions don't allow you to run a local server.

Although it might be tempting, I would avoid embedding a browser in pyfa as a lot of the security provided by OAuth (and the old EVE API) comes from not giving passwords to 3rd party apps, which is effectively what's happening with an embedded browser.

Authorizing an app is pretty low friction once a character is singed in to the EVE SSO, but CCP should probably consider increasing the token lifetime to cover a reasonable 3rd party app session length to avoid reauthorizing multiple times during a session.

@blitzmann
Copy link
Collaborator Author

Thanks for your input =) I've already got a server going that partially works - it extracts the token, but I haven't yet written the logic to get the access token. Shouldn't be too difficult. However I did not consider users that may be restricted by firewall settings. I'll have to give it some more thought on how to handle that situation.

What's your thought on shipping pyfa with the client key and secret vs having the user make one for themselves through the developer portal, in terms of security for both users and CCP? As I understand it, since we're talking about a single-user environment here, it should't be problematic unless the user database with their refresh token falls into malicious hands which can then use our client info with their refresh token to write to their fittings.

@jimpurbrick
Copy link

From an OAuth perspective pyfa is closer to a JavaScript client than a webserver (albeit with a more fiddly workflow for getting tokens) so I would use the Implicit Grant flow (https://tools.ietf.org/html/rfc6749#section-4.2).

While having pyfa pretend to be a webserver and using the Authorization Grant Flow (https://tools.ietf.org/html/rfc6749#section-4.1) might be tempting to get refresh tokens it would require users to sign up as a developer to get a client ID/secret, which isn't CCPs intention for the developer license and may require everyone using pyfa to have paid CCP for time/PLEX/fanfest/vegas/stuff.

If CCP can increase the lifespan of an implicit grant flow to something around the 90th percentile of pyfa session lengths (8 hours?) then you'll just need to click a single button in a browser when you start pyfa to get a token assuming you get the webserver working and people keep themselves signed in to the SSO, which isn't too onerous.

@blitzmann
Copy link
Collaborator Author

Implicit would indeed be the best way for pyfa to access the resources, however that 20 minute limit is pretty restrictive. It would be awesome if that was extended. The plan right now is to ship with our own client ID/Secret (as there's no way we could hide it), and possibly have the option for users to give their own if they wish.

I've thought of possibly giving user option to encrypt their refresh tokens (in case their database is compromised), and prompt them for decryption password when pyfa starts or the first time they attempt CREST access. But something such as this would probably not be in the initial release.

@regner
Copy link

regner commented Oct 18, 2015

I have been looking at bumping that 20 minute limit to 1 hour. Would that saw you enough?

@blitzmann
Copy link
Collaborator Author

I replied to you on Slack =)

@regner
Copy link

regner commented Oct 19, 2015

Going to try and keep the discussion here so if needed we can easily direct other people here.

Out of curiosity how to do imagine the user flow for this to be? Are you going to be letting them manage their EVE fittings or is the focus on just getting existing ones and putting new ones?

@blitzmann
Copy link
Collaborator Author

The intended flow would have been as such:

  1. User opens preferences or other CREST management window and hits "Add Character" which opens browser and directs them to SSO login
  2. User logs in, and is redirected to local webserver
  3. Character is added to a list of available characters and is saved to the DB along with refresh token. Login once and forget.

To export, I was gonna rework the current export interface which exports to clipboard to also give user dropdown menu of saved characters to POST to CREST. And to import: https://gyazo.com/cf6b5a47ffebc40717d4ac1316e64778

This kind of flow, where the characters are saved and are automatically authenticated, would require authorization grant with refresh tokens. But as stated on Slack after having thought about it, if user database is ever compromised then they are at risk of another party modifying their fits. Which is why I was thinking about encrypting refresh token based on user-supplied password. While I think this would be easier on the user as it will be quicker than opening a port and browser, signing in, and then going back to pyfa, I'm still not sure about it.

If the limit can be bumped up to 1 hour or more, I think that will be a good start. TBH, users will probably not be getting and posting fits all that often, and when they do it will probably be after a session of theorycrafting, so logging in when needed might not be too bad.

That said, sometimes it's not as simple as opening the browser and hitting "Authorize". I have to consider those that have multiple accounts. If someone has three accounts and wants to post some fittings to each, then they have to login three different times. This is where refresh tokens would definitely come in handy.

As for managing fittings, possible future CREST features must also be considered, such as Corp fittings and modifying existing fits (I plan to link pyfa fits to their EVE fitting so that that when CREST ).

So basically I have three options:

  1. Use Implicit Grant and have user login whenever they need to GET or POST. This may become more cumbersome if additional features are added in CREST such as modifying existing fits.
  2. Use Authentication Grant with pyfa's client ID/Secret and force refresh token encryption that user will unlock once during a session. Helps with fluid user interaction, and will help those with multiple accounts.
  3. Allow user to supply their own client ID/Secret which allows the benefits of using the refresh tokens without the risk.

Classic user security vs user interaction. /o\

@jimpurbrick
Copy link

This would be a great plan if the web server were hosted somewhere it could keep secrets and tokens safe. Hooking up with fleet-up or a similar service might be a good idea if you don't want to run your own servers.

@narthollis
Copy link

As previously discussed, the problem with using an external service to keep the secrets safe, is how do you verify that Pyfa is allowed to access those tokens?

Also, the web server that @blitzmann is referring to in his authentication work flow is a way of accepting a redirect from EVE SSO with the auth code. This web server would be embedded into Pyfa and is the way to get around messy protocol handlers (which no user shell handles consistently) or having the user copy/paste values into Pyfa.

If keeping the secret safe is the primary concern, then the Implicit grant path is really the only option.

Otherwise, I don't think there is any greater security risk storing the refresh tokens than storing the the api key/vcode pairs. (That said, an optional encryption is not a bad idea for those people who would like to use it)

@blitzmann
Copy link
Collaborator Author

Yeah, there would be no way of authenticating to a proxy server since it will also need a secret that cannot be kept secure; and we're back to square one =D

I think I will focus on using Implicit grant for default communications and allow users to put in their own client ID / Secret if they wish to do so, but I'll still keep the encryption as a possibility. Keeping our stuff secret is not a concern in and of itself being a single-user application, but since we frequently have people uploading their database for support / debugging, I can't allow their refresh tokens to be plaintext.

As for current progress, I have a very crude implementation on my repo. I hate the way I've put it together, so I'll be refining it in the coming weeks. But hey, it works!

@blitzmann
Copy link
Collaborator Author

Also, @regner, this sounds like an interesting solution that twitter came up with: https://groups.google.com/forum/#!topic/twitter-development-talk/wYrenYbIsjk

It's basically an official way for the user to create their own developer licence and generate their own application id / secret, which can then be sent to the callback and implemented in the application. Perhaps it might be something CCP would want to look at?

EDIT: oh, this process is actually now a draft protocol: OAuth 2.0 Dynamic Client Registration Protocol

https://tools.ietf.org/html/draft-ietf-oauth-dyn-reg-21

@blitzmann
Copy link
Collaborator Author

A working version has been pushed to the crest_fitting branch. There are two modes: implicit and user-defined client settings.

  • Under implicit, you can log in with one character at a time. This is mostly due to me being lazy and not wanting to write logic to juggle multiple characters with limited authentication time. Any CREST activity will be done under that character. The character expires after the time CCP sets (which hopefully @regner can get raised =D). There is a timer that is set that can execute some code when the expiration is met in case we need it.
  • Under user defined, user enters their own clientID and Secret. This will allow them to save all their logged in characters and select which one to use for CREST activity. This may help folks with multiple accounts, those that like to be able to select the character to work with, or those that simply have trust issues and want their own client info handling their data.

With both configurations, a local HTTP server is opened on port 6461. This will eventually be configurable in case other programs or firewalls cause issues.

There's still a few refinements that I wish to make, but I need to make sure the windows and OS X skeletons have the needed modules such as BaseHTTPServer.

Going forward past the initial release I will be linking fits that are imported and exported so that if CCP ever implements an "update" fit option we already have that linkage. For those with strict firewalls or administration issues preventing local servers, I will investigate making a separate clientID for pyfa that will point to a public callback that will extract and display the info needed, allowing the user to copy and paste this info into pyfa.

Known issues:

  • Changing clientID and Secret and trying to access CREST with refresh token bound to the old settings hangs the program. Still need to do some error management.
  • Changing from implicit to user-supplied mode probably doesn't work until reopening pyfa
  • (more of a todo): make sure that logoff timer is actually reset when changing characters under implicit mode

Screenshots

I'm open to suggestions on how best to display these infos.

Implicit mode, right after logging in. I don't like this, but it does show the current logged in character. I have been thinking of maybe adding this info to the Title bar or to a status bar on the main window.
implicitlogin

Implicit mode, exporting current fit to character (the text alignment is out of whack, but I also plan to incorporate this into the existing clipboard export dialog)
implicitexport

User mode, character management:
charmgmt

User mode, browse and import fits:
charbrowse

@narthollis
Copy link

Looks good.

I think the Implicit / User defined split is a good compromise to the secret problem, Nice work.

@blitzmann
Copy link
Collaborator Author

I've been working on the osx and windows skeletons, and should have working clients available for testing by tonight hopefully

More known issue:

  • was able to get a "Address already in use" when running two pyfas in OS X due to both trying to listen on same port. (FIXED by timing server out at 60 seconds)
  • The server binds to the address when pyfa starts (because it is done when the CREST service initializes). Move this out of init and only do it when startServer is called.
  • Character Info window doesn't pop up to foreground when spawned on OS X (FIXED by shoving that info in the title bar)

@cncfanatics
Copy link
Collaborator

The proper approach here is very likely to only start the server when
needed to get a response back ? That way two pyfas should be able to
co-exist nicely.

On Sun, Oct 25, 2015 at 8:51 PM, blitzmann [email protected] wrote:

I've been working on the osx and windows skeletons, and should have
working clients available for testing by tonight hopefully

More known issue:

  • was able to get a "Address already in use" when running two pyfas in
    OS X due to both trying to listen on same port.
  • The server binds to the address when pyfa starts (because it is done
    when the CREST service initializes). Move this out of init and only do it
    when startServer is called.


Reply to this email directly or view it on GitHub
#383 (comment).

@blitzmann
Copy link
Collaborator Author

Yep, already done =) The server also has a 60 second window before it's shut off.

Making a pre-release now. =D

@narthollis
Copy link

In regards to fallback, I think the best option is a basic middleman page.

CREST redirects to a public Pyfa webpage. The Pyfa webpage attempts to call back to the application by JSONP (to avoid potential cross origin issues). If this call is successful the middleman page tries to close its self, or failing that displays a "All done" message to the user.

The middleman page also displays the auth code to the user, with instructions on how to paste it into Pyfa via a fall back method.

The middleman does nothing with the auth code other than pass it to the application or displays it to the user.

Being basic HTML/JS this page could be hosted with GitHub pages.

@blitzmann
Copy link
Collaborator Author

I was thinking of exactly that, but was waiting to look into hosting. I totally forgot about GitHub pages though!

@jimpurbrick
Copy link

crest explorer and contactjs are both hosted on github pages and that works
well

On 25 October 2015 at 20:48, blitzmann [email protected] wrote:

I was thinking of exactly that, but was waiting to look into hosting. I
totally forgot about GitHub pages though!


Reply to this email directly or view it on GitHub
#383 (comment).

@blitzmann
Copy link
Collaborator Author

Merged CREST support with singularity branch

@regner
Copy link

regner commented Nov 3, 2015

@Nothing4You
Copy link

Just tested this on TQ.
Created new application: Settings

However, when I try to add a character (after saving the settings in pyfa) I see https://login.eveonline.com/oauth/authorize?response_type=code&redirect_uri=http://localhost:6461&client_id=&scope=characterFittingsRead+characterFittingsWrite&state=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx in my browser (note that the client_id is missing) telling me {"error":"invalid_request", "error_description":"Some parameters are either missing or invalid"}

@Nothing4You
Copy link

Tried again now and I got a new error with a wrong url due to the wiki being wrong (already corrected).

Now that I fixed the URL I can add a character.

EDIT: ClientID is being added now, no idea why it didn't get added when I tried first.

@Nothing4You
Copy link

Another update: I wasn't able to do anything with the characters I added until I restarted pyfa.

@blitzmann
Copy link
Collaborator Author

Dammit, I meant to change those hardcoded links. fml

Will hopefully get a point release out tonight

@blitzmann
Copy link
Collaborator Author

https://github.com/DarkFenX/Pyfa/releases/tag/v1.16.1

Should fix the TQ CREST issues, thanks @regner

@Nothing4You: Yeah, switching between the modes doesn't work very well at the moment. It's fortunate that it's not needed often. I have no idea why clientID would be missing, unless it's another side effect of switching from Implicit to User defined.

EDIT: NVM I see what you mean, The menu item is disabled eve after adding characters. Will put it on the list =)

@blitzmann
Copy link
Collaborator Author

I've done a bit more work on the logic of the menus - they should now correctly react to logging in, out, and switching modes.

@noenedrops
Copy link

Notice: authentication started working after I restarted pyfa.

Old post:

Chiming in to add that in 1.16.2/osx/wx3 CREST integration via user-supplied details is still not working for me.

First, if I provide credentials in settins, then go to the menu CREST > Manage Characters > Add Character, pyfa opens authentication URL in the browser without correctly inserting the Client ID from settings to the callback URL, it remains like this:

https://login.eveonline.com/oauth/authorize?response_type=code&redirect_uri=http://localhost:6461&client_id=&scope=...

and CCP's OAuth reponds with

{"error":"invalid_request", "error_description":"Some parameters are either missing or invalid"}

Second, even if I fix the URL manually with specifying my ClientID param, it redirects me to the auth page where I press 'Authorize' button and it redirects me back to localhost, which the browser cannot access, because "Server not found". After trying this back and forth a couple of times pyfa's localhost server has finally responded with "all done, you may close this window", but my character did not appear in the character management window.

@thorr18
Copy link

thorr18 commented Nov 9, 2015

@noenedrops , I get a long URL with client_id included.
I opened 1.16.2/osx/wx3 , clicked CREST , clicked Login To Eve, Chrome browser popped up displaying Chrome saved credentials, clicked log in, clicked authorize, closed browser, exported a fit to Eve in Pyfa, and verified it appeared on my Eve character.

@blitzmann
Copy link
Collaborator Author

@thorr18, I believe @noenedrops is talking about switching to user-supplied client details. I'll have to look at the os x client again and see if I can duplicate.

@noenedrops, The server only listens about 60 seconds after the initial request, which may be why you were getting a not found error. If it says "done" but the character isn't added, it might be because the state is not correct. Run with the -d flag to print debug messages, which may help diagnose exactly what is going on. (but honestly, the response handling here is barebones. I really need to improve on it)

@blitzmann
Copy link
Collaborator Author

@noenedrops, you are clicking the "Apply client settings" button after inserting your info, correct?

@blitzmann
Copy link
Collaborator Author

CREST is dead, long live ESI!

Closing this, all enhancements / todos / whatever revolving around ESI should be handled in separate issues as they crop up. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is a feature request, or an idea to enhancement a current feature
Projects
None yet
Development

No branches or pull requests

8 participants