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 auth to omit content-type headers and body #23

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

adamzuyang
Copy link
Contributor

@adamzuyang adamzuyang commented Nov 17, 2024

Describe the bug
Symphony recently made a release that changed how an invalid request is handled. This causes issues with session creation.

To Reproduce
Sending empty body to POST /sessionauth/v1/authenticate results in either a 401 response (if a content-type header is sent) or a 500 response (if no header is sent).

E.g., the following results in a 401 response:

def _client_cert_post(host: str, request_url: str, cert_file: str, key_file: str) -> str:
    request_headers = {"Content-Type": "application/json"}

    context = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
    context.load_cert_chain(certfile=cert_file, keyfile=key_file)
    connection = http.client.HTTPSConnection(host, port=443, context=context)
    connection.request(
        method="POST",
        url=request_url,
        headers=request_headers,
        body=json.dumps({}),
    )
    response = connection.getresponse()

    if response.status != 200:
        print(response.read().decode("utf-8"))
        raise Exception(
            f"Cannot connect for symphony handshake to https:{host}{request_url}: {response.status}:{response.reason}"
        )
    data = response.read().decode("utf-8")

    return json.loads(data)

Expected Behavior
We expect tokens to be returned as in https://rest-api.symphony.com/main/bot-authentication/session-authenticate

Error Message
401 or 500 response from Symphony

Runtime Environment
CSP: 0.0.5
SYS: 3.11.9 | packaged by conda-forge | (main, Apr 19 2024, 18:36:13) [GCC 12.3.0]
SYS platform: Linux

Additional context
Without authentication tokens, this adapter becomes unusable with certificate and key.

@adamzuyang adamzuyang changed the title Update auth to omit content-type headers and body Update auth to omit content-type headers and body --signoff Nov 18, 2024
@adamzuyang adamzuyang changed the title Update auth to omit content-type headers and body --signoff Update auth to omit content-type headers and body Nov 18, 2024
@adamzuyang adamzuyang force-pushed the patch-1 branch 2 times, most recently from 6589ac8 to 27f5317 Compare November 18, 2024 12:22
NeejWeej
NeejWeej previously approved these changes Nov 18, 2024
@NeejWeej
Copy link
Collaborator

From here: https://rest-api.symphony.com/main/bot-authentication/session-authenticate#session-token-management

It says:

The token you receive is valid for the lifetime of a session that is defined by your pod's administration team. This ranges from 1 hour to 2 weeks.

You should keep using the same token until you receive a HTTP 401, at which you should re-authenticate and get a new token for a new session.

Datafeeds survive session expiration, you do not need to re-create your datafeed if your session expires.

I think it might be worthwhile to incorporate a retry here (and probably also for the other normal checks) so that bots can last past session expiration.

@robambalu
Copy link
Collaborator

@adamzuyang I agree with @NeejWeej but at the same time I'm ok getting this patch out now since symphony is rolling out a breaking authentication change on their end. @NeejWeej please open a separate issue to track.
@adamzuyang please fix the lint errors so that we can get this in

@timkpaine timkpaine merged commit d926d1b into Point72:main Nov 20, 2024
3 checks passed
@timkpaine timkpaine added the type: enhancement Minor improvements label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Minor improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants