pyfa-org / Pyfa

Python fitting assistant, cross-platform fitting tool for EVE Online
GNU General Public License v3.0
1.6k stars 406 forks source link

Update the SSO Login for Serenity and Singularity server's player #2437

Closed huangzheng2016 closed 6 months ago

huangzheng2016 commented 2 years ago

I created a setting for the different servers image Now,Serenity and Singularity player can login from their own SSO system image

blitzmann commented 2 years ago

Hey there., thanks for the PR!

So, I have a lot of questions, and I'm relying on you as I don't have the ability to test myself since I do not have a Serenity account. :)

1) Is there any documentation on how OAuth works for the serenity server? It looks like it's different. 2) API_CLIENT_ID_SERENITY where is this registered? The ClientID is something we as pyfa devs have created (current one is linked to my personal account). If we're introducing a new client ID that a third party (yourself) has made, I don't know what the implications of that are (if any) or if there's any security implications regarding this. 3) As to the first point, the flow is confusing. We're redirecting to SSO_CALLBACK_SERENITY='https://esi.evepc.163.com/ui/oauth2-redirect.html', and I assume this has the authorization code in the URL. Looks like we're pulling the code from the manual input option (which would otherwise have a base6t4-encoded message with some additional information) 4) The login request sends the following parameters: 'state': 'hilltech', 'device_id': 'eims'. What do these values signify? State is supposed to ensure that the application returns back to the proper state after redirect (pyfa uses it as a simple verification guid), but I have no idea what device_id is or how it's used. Is this something that is required for Serenity's SSO Login? 5) Similar to the above, there is no code challenge. This leads me to believe that this is using implicit grant authentication, and a refresh token is not obtained. Is that correct?

If you could annotate the various changes you've made to help me better understand the reasoning behind them, that would be great. again, I cannot test myself, and so I want to fully understand the decisions made before attempting to merge.

Thanks for you assistance with this! :)

huangzheng2016 commented 2 years ago

First of all, I can provide my account for you to test.If you need it, please email me.

I refer to the development documentation in Tranquility as well.

  1. This documentation is only https://esi.evepc.163.com/ui/, Serenity does not open applications for API_CLIENT_ID,but an official test Web application is provided.

  2. EVE Swagger UI is the demo of Serenity server,you can try it yourself

image image I extract the API_CLIENT_ID as API_CLIENT_ID_SERENITY.This is one of the alternatives and is currently the only method for Serenity Server SSO Login until officially open applications

  1. SSO_CALLBACK_SERENITY is just the CALLBACK of EVE Swagger UI,As a Web application, it can't receive a CALLBACK when it's not open, so we end up extracting the URL from a blank page and typing it into Pyfa.The URL contains the code required to obtain the access_token and refresh_token. This code is one-time and valid for 5 minutes

  2. device_id is extracted from the EVE Swagger UI. Beacuse code challlenge notsupported by Serenity SSO state, so you can just full it with some word. It will return the same words you asked for.

  3. No, you can use the user input the URL of the extracted code, send code and client_id to the https://login.evepc.163.com/v2/oauth/token for refresh_token, this process when a one-off.Except for code challenge and login url, everything else is similar to Tranquility

huangzheng2016 commented 2 years ago

I have sent this Dev version to my company employees for testing, and everything works normally, which is the same as most alliance ESI authorization process

blitzmann commented 2 years ago

@huangzheng2016 I've pushed a small fix where the client IDs were getting mixed up.

I was able to successfully login using the credentials you provided - thank you for that.

image

Seems to work as well. I'm going to evaluate it a little more closely since the oauth flow is a bit different from what I'm used to (refresh token available without any client secret?!). Additionally, I think it makes sense to store the server along with the ESI character, in case... people have accounts on multiple servers? /shrug

Good work on this initial effort tho, works really well :)

huangzheng2016 commented 2 years ago

Thank you for the repair. I do think it will be possible to bind servers to ESI characters in the future. Serenity's oauth is difference from Tranquility, but I think this is the most convience ones that I have knew.

blitzmann commented 2 years ago

I looked into adding the option to save the server with the character. The proper route would be a bit difficult, considering the ESI service, once initialized, uses whatever the settings have for the server at the time of initialization, and then it doesn't refresh them. To make API calls with respect to the characters server, we would have to make sure that ESI service is aware of that and re-initializes the data it needs for that specific server. This would require a bit of refactoring on that service to achieve that. I'm going to look more into it a bit in the next couple days.

I do think that this is something we need to address now instead of in the future. I can envision folks having SISI characters and trying to update skills for that character, but the server selected is TQ, and it just gets confusing lol.

huangzheng2016 commented 2 years ago

Haha.I'm looking forward to your good news.

blitzmann commented 2 years ago

I've updated the branch to support saving server alongside SSO character. I still need to force manual login type if using Serenity, since the server will not receive the callback.

Additionally, I've disabled SISI option since apparently CCP got rid of SISI ESI a couple of years ago, so there's no point in allowing logins to it

blitzmann commented 2 years ago

I've made various updates tot he SSO login in general, with it now accepting the manual token input regardless of if the server is turned on in the settings. Along with this was a slight rework of how exceptions are handled.

There's an issue that I'm running into at the moment which has the whole program crash when exiting the preferences window. No idea what's going on there.

huangzheng2016 commented 2 years ago

When I have time, I will test why it will exit

blitzmann commented 2 years ago

I cannot get the crash to happen anymore. Wondering if it was my settings getting corrupted as I was working on some things. Seems to work now.

Gonna do a bit more testing, bu this is close to being ready. @huangzheng2016 would like you to follow up with some testing as well from your end :)

copyliu commented 2 years ago

@blitzmann

1. Is there any documentation on how OAuth works for the serenity server? It looks like it's different.

I writed some notes about Serenity server OAuth : https://github.com/copyliu/esi-docs/blob/magic_serenity/docs/sso/web_based_sso_flow.md

4. The login request sends the following parameters: `'state': 'hilltech', 'device_id': 'eims'`. What do these values signify? State is supposed to ensure that the application returns back to the proper state after redirect (pyfa uses it as a simple verification guid), but I have no idea what `device_id` is or how it's used. Is this something that is required for Serenity's SSO Login?

device_id is a NetEase implement for their security control, about how to get a device_id see doc before, but in fact device_id can be a random string.

5. Similar to the above, there is no code challenge. This leads me to believe that this is using implicit grant authentication, and a refresh token is not obtained. Is that correct?

there some security issue about code challenge/access token endpoint from CCP, these security issues was reported to CCP team years ago, as result, EVE SSO can return refresh token without code challenge AND client secret

huangzheng2016 commented 8 months ago

The thing is exactly as @copyliu said. But it seems I need to update it to adapt to the latest pyfa, maybe consider this PR

huangzheng2016 commented 7 months ago

@blitzmann fix the bug and work well but cached_session in esi.py

huangzheng2016 commented 6 months ago

new one https://github.com/pyfa-org/Pyfa/pull/2590