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

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

Closed
wants to merge 0 commits into from

Conversation

huangzheng2016
Copy link
Contributor

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

@blitzmann
Copy link
Collaborator

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
Copy link
Contributor Author

huangzheng2016 commented Apr 29, 2022

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
Copy link
Contributor Author

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
Copy link
Collaborator

@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
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

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

@blitzmann
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Contributor Author

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

@blitzmann
Copy link
Collaborator

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
Copy link
Contributor

copyliu commented May 18, 2022

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

huangzheng2016 commented Feb 11, 2024

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

@huangzheng2016
Copy link
Contributor Author

new one #2590

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

Successfully merging this pull request may close these issues.

3 participants