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

OAuth support #34

Open
wants to merge 57 commits into
base: master
Choose a base branch
from
Open

OAuth support #34

wants to merge 57 commits into from

Conversation

hughandersen
Copy link

OAuth IBKR code added to repo.
Work to make functions to call IBKR via local host or OAuth needs to be developed.

@hughandersen hughandersen reopened this Nov 3, 2024
@hughandersen
Copy link
Author

First commit

@hughandersen hughandersen reopened this Nov 3, 2024
@Voyz
Copy link
Owner

Voyz commented Nov 4, 2024

Okay, I saw that you started with changing all the methods in mixins. I appreciate the effort but I don't think that will be necessary. If we look at the send_oauth_request:

 def send_oauth_request(
        request_method: str,
        request_url: str,
        oauth_token: str | None = None,
        live_session_token: str | None = None,
        extra_headers: dict[str, str] | None = None,
        request_params: dict[str, str] | None = None,
        signature_method: str = "HMAC-SHA256",
        prepend: str | None = None,
    ) -> requests.Response:
    headers = {
        "oauth_consumer_key": consumer_key,
        "oauth_nonce": generate_oauth_nonce(),
        "oauth_signature_method": signature_method,
        "oauth_timestamp": generate_request_timestamp(),
    }

    if oauth_token:
        headers.update({"oauth_token": oauth_token})
    if extra_headers:
        headers.update(extra_headers)
    base_string = generate_base_string(
        request_method=request_method,
        request_url=request_url,
        request_headers=headers,
        request_params=request_params,
        prepend=prepend,
    )
    logger.info(
        msg={
            "message": "generated base string",
            "timestamp": time.time(),
            "details": {
                "base_string": base_string,
                "request_method": request_method,
                "request_url": request_url,
                "request_headers": headers,
                "request_params": request_params,
                "prepend": prepend,
            },
        }
    )
    if signature_method == "HMAC-SHA256":
        headers.update(
            {
                "oauth_signature": generate_hmac_sha_256_signature(
                    base_string=base_string,
                    live_session_token=live_session_token,
                )
            }
        )
    else:
        headers.update(
            {
                "oauth_signature": generate_rsa_sha_256_signature(
                    base_string=base_string,
                    private_signature_key=read_private_key(
                        signature_key_fp
                    ),
                )
            }
        )
    logger.info(
        msg={
            "message": "generated signature",
            "timestamp": time.time(),
            "details": {
                "signature": headers["oauth_signature"],
                "signature_method": signature_method,
            },
        }
    )
    response = requests.request(
        method=request_method,
        url=request_url,
        headers={
            "Authorization": generate_authorization_header_string(
                request_data=headers,
                realm=realm,
            )
        },
        params=request_params,
        timeout=10,
    )
    logger.info(
        msg={
            "message": "sent oauth request",
            "timestamp": time.time(),
            "details": {
                "request_method": request_method,
                "request_url": response.request.url,
                "request_headers": response.request.headers,
                "request_body": response.request.body,
                "response_status_code": response.status_code,
                "response_error_message": response.text if not response.ok else None,
            },
        }
    )
    return response

You'll notice that all it essentially does is set some headers on the request. Then there's some logging, but we don't need that, or we could do it elsewhere.

Hence, in reality, all we need to do to implement this into the current version of IBind, is to expand our existing request flow to attach these headers.

To start, we need to add a empty get_headers method to RestClient. We will later override it in IbkrClient:

def get_headers(        
        request_method: str,
        request_url: str,
        request_params: dict[str, str] | None = None,
        ):    
    return None

In the RestClient's requestmethod:

def request(self, method: str, endpoint: str, attempt: int = 0, log: bool = True, **kwargs) -> Result:

We can call the get_headers and then attach them when making request here:

response = requests.request(method, url, verify=self.cacert, timeout=self._timeout, **kwargs)


Then in the IbkrClient, we override the get_headers method, and generate the headers similar to how it happens in the send_oauth_request:

def get_headers(        
        request_method: str,
        request_url: str,
        request_params: dict[str, str] | None = None,
        ):    
    
    # TODO: read or generate all necessary variables here

    headers = {
        "oauth_consumer_key": consumer_key,
        "oauth_nonce": generate_oauth_nonce(),
        "oauth_signature_method": signature_method,
        "oauth_timestamp": generate_request_timestamp(),
    }

    if oauth_token:
        headers.update({"oauth_token": oauth_token})
    if extra_headers:
        headers.update(extra_headers)
    base_string = generate_base_string(
        request_method=request_method,
        request_url=request_url,
        request_headers=headers,
        request_params=request_params,
        prepend=prepend,
    )
    if signature_method == "HMAC-SHA256":
        headers.update(
            {
                "oauth_signature": generate_hmac_sha_256_signature(
                    base_string=base_string,
                    live_session_token=live_session_token,
                )
            }
        )
    else:
        headers.update(
            {
                "oauth_signature": generate_rsa_sha_256_signature(
                    base_string=base_string,
                    private_signature_key=read_private_key(
                        signature_key_fp
                    ),
                )
            }
        )
    return {
            "Authorization": generate_authorization_header_string(
                request_data=headers,
                realm=realm,
            )
        }

That method does require us to provide the oauth_token and consumer_key - this seems quite easy, I assume we can just read them from environment variables.

It also requires a live_session_token, which we can generate automatically on the fly. We use the same methods from IBKR, but run them automatically when we call get_headers method.

Within get_headers we can also check fo a new member variable of IbkrClient called self._use_oauth that we pass as a constructor parameter. If it is false, we return None. This would allow us to maintain the existing authentication flow. But if self._use_oauth is True, we go ahead and generate the live_session_token and subsequently the headers.


This way, all extra authentication should happen automatically for the user, and all existing methods will be able to stay the same. We'd just have to set the correct environment variables and specify that we wanna use OAuth when constructing the IbkrClient:

ibkr_client = IbkrClient(..., use_oauth=True)

Let me know if you can see some roadblocks in this implementation or if you'd like to discuss anything before giving it a crack. Otherwise, let me know if you get stuck anywhere and I'll be happy to help.

@hughandersen
Copy link
Author

That sounds like a good plan, I'll update the code and commit again for review shortly.

@hughandersen
Copy link
Author

hughandersen commented Nov 8, 2024

Voy, I've started to make the changes you suggested, let me know if I understand you code correctly and how they look.

The first step gets a live_session_token and access_token if the user sets use_oauth=True.

self._use_oauth=use_oauth
self.live_session_token,self.access_token=(self.req_live_session_token if self._use_oauth else None,None)]

But the call to post, and then request functions in rest_client.py need checking.

response=self.post(self, path=REQUEST_URL, params = extra_headers,prepend=prepend, log = True)

Once authorized, subsequent calls to endpoints will get the headers in the request function, but it needs some work.

header=self.get_headers(request_method=method,request_url=endpoint,request_params=TBD)

@Voyz
Copy link
Owner

Voyz commented Nov 11, 2024

Hey @hughandersen great job making the progress! Just to start reviewing it, I'm gonna ask you to add .venv to .gitignore file and remove it from GitHub. It's usually not a good idea to include these in a public repo. Any kind of local files like these, node-modules, .idea, etc. Then I'll be able to give it a review, as currently it's 1000s of files that are slowing down the PR review page

@hughandersen
Copy link
Author

@Voyz Sure, my mistake, I forgot to check that the gitignore file excludes .venv before committing. Do you have a preferred method to remove the folder?

@Voyz
Copy link
Owner

Voyz commented Nov 11, 2024

No problem at all 👍 Just the normal way, remove them from git but keep them locally:

https://stackoverflow.com/questions/1143796/remove-a-file-from-a-git-repository-without-deleting-it-from-the-local-filesyste
https://stackoverflow.com/questions/936249/how-to-stop-tracking-and-ignore-changes-to-a-file-in-git

@hughandersen
Copy link
Author

@Voyz Done, check and let me know if the folder hasn't been removed successfully.

@hughandersen
Copy link
Author

hughandersen commented Nov 11, 2024

@Voyz Can you take a look at the file rest_08_oauth.py in examples?
I usually use the debug function in an interactive cell to find problems, but I get an error

ModuleNotFoundError                       Traceback (most recent call last)
File d:\git_repos\ibind\examples\rest_08_oauth.py:4
      [1](file:///D:/git_repos/ibind/examples/rest_08_oauth.py:1) breakpoint()
      [2](file:///D:/git_repos/ibind/examples/rest_08_oauth.py:2) #%%
----> [4](file:///D:/git_repos/ibind/examples/rest_08_oauth.py:4) from ibind.client.ibkr_client.py import IbkrClient
      [7](file:///D:/git_repos/ibind/examples/rest_08_oauth.py:7) # Construct the client, set use_oauth=False, if working, try creating a live session by setting use_oath=True
      [8](file:///D:/git_repos/ibind/examples/rest_08_oauth.py:8) client = IbkrClient(use_oauth=False)

ModuleNotFoundError: No module named 'ibind.client.ibkr_client.py'; 'ibind.client.ibkr_client' is not a package

Is this something to do with the __init__.py file?
Once this is working I can try the code and fix the bugs.

@Voyz
Copy link
Owner

Voyz commented Nov 11, 2024

Better! Try doing the same with .vscode

@Voyz
Copy link
Owner

Voyz commented Nov 11, 2024

To be able to run it you need to add the root directory to PYTHONPATH:

https://www.simplilearn.com/tutorials/python-tutorial/python-path
https://stackoverflow.com/questions/41471578/visual-studio-code-how-to-add-multiple-paths-to-python-path
https://stackoverflow.com/questions/3402168/permanently-add-a-directory-to-pythonpath

You shouldn't need to change:

from ibind import IbkrClient to from ibind.client.ibkr_client.py import IbkrClient

Try that and let me know if it helps.

@hughandersen
Copy link
Author

@Voyz Voy the path has been fixed (simple typo) and I'm working on getting live session and access tokens.
Can you review the code and add any suggestions/improvements?

Copy link
Owner

@Voyz Voyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hughandersen superb job on the first iteration of this PR 👏👏 You've followed the suggestions I made very well and did an amazing job resolving the module import / pythonpath issues you faced initially (I hate these too 🙄).

I've left a number of comments for you to address in order to do the next few steps towards getting this merged in, but honestly you're on the right path and are doing really well so far. Thanks!

.gitignore Show resolved Hide resolved
examples/rest_08_oauth.py Outdated Show resolved Hide resolved
examples/rest_08_oauth.py Outdated Show resolved Hide resolved
examples/rest_08_oauth.py Outdated Show resolved Hide resolved
ibind/base/rest_client.py Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
OAuth/How to implement OAuth.md Outdated Show resolved Hide resolved
OAuth/first-party-oauth-main/requirements.txt Outdated Show resolved Hide resolved
ibind/base/rest_client.py Outdated Show resolved Hide resolved
ibind/client/ibkr_client.py Outdated Show resolved Hide resolved
@hughandersen
Copy link
Author

OAuth session initialization (getting live session and access tokens) is now working. But getting the brokerage session established is not working yet (see examples/rest_09_oauth_session.py).
Once brokerage method is working I'll clean up the code.

@hughandersen
Copy link
Author

hughandersen commented Nov 25, 2024

Code can now get live_session_token,live_session_token_expires_ms data, and get market data using original ibind methods, see file rest_08_oauth_test.py.
To do:

  • either use a .env file or os environment to store variables
  • there is a conflict using **kwargs in line 183 of file rest_client.py
response = requests.request(method,url,headers=header_oauth,params=params,timeout=10)
# response = requests.request(method, url, verify=self.cacert,headers=header_oauth,params=params, timeout=self._timeout, **kwargs)
  • endpoints need to be tested more thoroughly

@Voyz take a look when you get a chance

Copy link
Owner

@Voyz Voyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good progress @hughandersen 👏👏 I've left a few comments.

Do I understand you can communicate with the IBKR API now correctly using OAuth? Calling various endpoints in the example file works?

I didn't review the oauth_requests.py and get_session.py as these seem to be your WIP files - do I understand correctly?

ibind/base/rest_client.py Outdated Show resolved Hide resolved
super().__init__(url=url, cacert=cacert, timeout=timeout, max_retries=max_retries)

if self._use_oauth:
self.live_session_token,self.live_session_token_expires_ms=self.req_live_session_token()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great 👏 I understand these can get generated just once per session and don't need to be re-generated?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. The live session token times out after 24hours, and I think it would need to be re-created if the session drops out. Maybe also allow the live session token to be re-created in a function?
It took some fiddling to get the order of the attributes correct (super before if self._use_oauth etc.), can you see any room for improvement?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well we do get that live_session_token_expires_ms value right - that should tell us when it expires?

Or even better - we could observe what does IBKR return back when we try to call its endpoints with an expired live_session_token. Then we catch that error and request a new live session token when it happens. No need to keep track of the expiry date, just handle it once it happens. What do you recon?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I could check what get returns after the expiry date, but I think there are other bigger bugs to fix right now.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Can you add a TODO comment in here so that we remember to handle this later? We shouldn't release until we have a nice way to handle it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I'll add a comment.

.vscode/settings.json Outdated Show resolved Hide resolved

#%%
# get brokerage session
brokerage_session_response=client.initialize_brokerage_session(publish='true',compete='true')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to call this for things to work? If so, how about we do it automatically for the user?

Copy link
Author

@hughandersen hughandersen Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, but the IBKR document is unclear, and I needed to call it before calling the other endpoints.
I'll check with them.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice initiative! Though I'd say just comment this line out and see if it works. If it does, then leave it out, if it doesn't leave it in 👍

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've emailed IBKR, lets see what they come back with.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hughandersen what's the status on this? Do we know if we need to call this? What happens if we don't? I see that you already make several requests before calling this one - do they succeed? If so, can we remove it?

ibind/client/ibkr_client_mixins/oauth_mixin.py Outdated Show resolved Hide resolved
@thouseef
Copy link

thouseef commented Jan 15, 2025

This is the link I was given, is there a new one?
[https://ndcdyn.interactivebrokers.com/sso/Login?action=OAUTH&RL=1&ip2loc=US](https://ndcdyn.interactivebrokers.com/sso/%5BLogin%5D(https://ndcdyn.interactivebrokers.com/sso/Login?action=OAUTH&RL=1&ip2loc=US)?action=OAUTH&RL=1&ip2loc=US)

I think it is the same :
https://ndcdyn.interactivebrokers.com/sso/Login?action=OAUTH is the one I used.

@Voyz
Copy link
Owner

Voyz commented Jan 15, 2025

@janfrederik please see ibind[oauth]==0.1.10-rc12 for the oauth_rest_url implemented as OAuthConfig parameter, as per your suggestions

@defisapiens
Copy link

I'm using ibind[oauth]==0.1.10-rc12 and trying to place orders using the code from examples/rest_04_place_order.py but it always returns the error:

2025-01-15 18:09:30 - INFO - POST https://api.ibkr.com/v1/api/iserver/account/UXXXXXXX/orders {'json': {'orders': [{'conid': 538132976, 
'side': 'BUY', 'quantity': 7, 'orderType': 'MKT', 'cOID': 'my_order-20250115180930', 'acctId': 'UXXXXXXX', 'tif': 'GTC'}]}}
Error occurred while executing buy order: IbkrClient: response error Result(data=None, request={'url': 'https://api.ibkr.com/v1/api/iserver/account/UXXXXXXX/orders', 'json': {'orders': [{'conid': 538132976, 'side': 'BUY', 'quantity': 7, 'orderType': 'MKT', 'cOID': 'my_ord
er-20250115180930', 'acctId': 'UXXXXXXX', 'tif': 'GTC'}]}}) :: 400 :: Bad Request :: {"error":"Bad Request: no bridge","statusCode":400}

Login through OAuth is working though. Has anyone faced such an issue?

@hughandersen
Copy link
Author

@defisapiens 'no bridge' sounds like an IBeam error, did you connect to the API using OAuth?

@defisapiens
Copy link

defisapiens commented Jan 16, 2025

to

I guess i did as IBIND_USE_OAUTH is set to True in the environment and Live session token is shown upon IbkrClient initialization. Could the issue have something to do with a missing cacert, as i do not provide one?

@salsasepp
Copy link

Login through OAuth is working though. Has anyone faced such an issue?

Did you start a brokerage session, like so?
client.initialize_brokerage_session(publish=True, compete=True)

@hughandersen
Copy link
Author

@defisapiens I don't think it's due to the cacert missing. If you can get other endpoints to work, like positions, then there is something wrong with the code to place orders. I'll try once my keys have been reset.

@defisapiens
Copy link

defisapiens commented Jan 16, 2025

Login through OAuth is working though. Has anyone faced such an issue?

Did you start a brokerage session, like so? client.initialize_brokerage_session(publish=True, compete=True)

Oh, i've missed that one. I'll try it out, thank you.

@janfrederik
Copy link

@janfrederik please see ibind[oauth]==0.1.10-rc12 for the oauth_rest_url implemented as OAuthConfig parameter, as per your suggestions

@Voyz Perfect. Works fine :-)

@janfrederik
Copy link

Login through OAuth is working though. Has anyone faced such an issue?

Did you start a brokerage session, like so? client.initialize_brokerage_session(publish=True, compete=True)

Oh, i've missed that one. I'll try it out, thank you.

@defisapiens @salsasepp
For me, it's working after doing client.initialize_brokerage_session(publish=True, compete=True).

@Voyz Would be good to document the need for this command more clearly in the doc than somewhere hidden in the wiki (https://github.com/Voyz/ibind/wiki/API-Reference-%E2%80%90-IbkrClient#client.ibkr_client_mixins.session_mixin.SessionMixin.initialize_brokerage_session). I would suggest to at least mention on the "Basic Concepts" page that this is necessary for the /iserver endpoints (market data, orders, ...).
It is not in the examples either: examples rest_04_place_order.py and rest_05_marketdata_history.py give the same error (when removing the mocking). Or do we only get this "bridge"-error when using oauth?
Anyway, good to mention that the error 400 :: Bad Request :: {"error":"Bad Request: no bridge","statusCode":400} means that client.initialize_brokerage_session(publish=True, compete=True) should be called.

@Voyz
Copy link
Owner

Voyz commented Jan 17, 2025

@janfrederik naturally, another item for the new wiki. My understanding is that the initialize_brokerage_session endpoint doesn't need to be called when OAuth isn't used, hence putting it in Basic Concepts would make more sense once OAuth becomes the main or only mean of connecting. At the moment OAuth WiKi would be more appropriate, along with a new Troubleshooting page where we could mention this error in particular. Same logic applies to examples. Thanks for the suggestions 👍

@thouseef
Copy link

Today I was trying to get market data history and got Request timeout a few times. Then I increased the timeout to 20 and got the data a few times. After few more tries, now I'm getting 503 Service unavailable.

ibind.support.errors.ExternalBrokerError: IbkrClient: response error Result(data=None, request={'url': 'https://api.ibkr.com/v1/api/iserver/marketdata/history', 'params': {'conid': 320227565, 'bar': '1min', 'period': '1min', 'outsideRth': True}}) :: 503 :: Service Unavailable :: {"error":"Service Unavailable","statusCode":503}

@thouseef
Copy link

On further reading, seems like if IBKR takes more than 10s to process a request, they return 503 as the status. Setting the timeout in iBind back to 10s resulted in a bunch of Request timeout, but did provide me with marketdata history for few other symbols.

@hughandersen
Copy link
Author

@Voyz Fyi IBKR also have OAuth2 for business clients, which is easier to use.
Is it worth more pain to implement OAuth2 as well?

@Voyz
Copy link
Owner

Voyz commented Jan 21, 2025

@thouseef thanks for reporting - where did you read that 503 is returned if their server takes more than 10s to respond?

@Voyz
Copy link
Owner

Voyz commented Jan 21, 2025

@hughandersen thanks for bringing it up - yes, potentially at some point we could look into it, but not as a part of this PR. Do I understand correctly it is pretty much ready to merge? Did it work for you after getting back from your holidays?

@thouseef
Copy link

@thouseef thanks for reporting - where did you read that 503 is returned if their server takes more than 10s to respond?

https://www.interactivebrokers.com/campus/ibkr-api-page/webapi-ref/#tag/Trading-Market-Data/paths/~1iserver~1marketdata~1history/get

Read the explanation for status 503
Every other instance of 503 also has the 10s timeout mentioned.

@salsasepp
Copy link

@thouseef thanks for reporting - where did you read that 503 is returned if their server takes more than 10s to respond?

I believe 503 happened to me also a few times, when calling search_strikes_by_conid for SPX options and the front month. In every case the default retry built into RestClient took care of that, worked on 2nd try.
@Voyz rc12 has been working here for a couple of days now, contract info and marketdata snapshots mostly, no issues

@Voyz
Copy link
Owner

Voyz commented Jan 21, 2025

@salsasepp RestClient managed to retry when getting 503s? That's unexpected, it should only retry on ReadTimeout, though I'm glad it helped. Next time this happens, would you mind sharing the log, possibly in a new issue?

@salsasepp
Copy link

@salsasepp RestClient managed to retry when getting 503s? That's unexpected, it should only retry on ReadTimeout, though I'm glad it helped. Next time this happens, would you mind sharing the log, possibly in a new issue?

@Voyz Thanks, will do. Can't reproduce it right now. You're correct of course, a 503 should not retry automatically. I might not remember correctly.

@hughandersen
Copy link
Author

@hughandersen thanks for bringing it up - yes, potentially at some point we could look into it, but not as a part of this PR. Do I understand correctly it is pretty much ready to merge? Did it work for you after getting back from your holidays?

@Voyz I agree, just letting everyone know about OAuth2. IBKR had a problem with my OAuth1 settings and we had to move them from AU server to US server, so I need to wait for the weekend reset and try again on Monday. I'll let you know.

@Voyz
Copy link
Owner

Voyz commented Jan 21, 2025

@hughandersen ah damn it, what a shame. I hope it can work again. Does that mean you didn't get a chance to test it since you got back?

@hughandersen
Copy link
Author

hughandersen commented Jan 21, 2025

@Voyz I did briefly, and the code worked, but then the connection broke..

@Voyz
Copy link
Owner

Voyz commented Jan 22, 2025

@hughandersen ok gotcha. Seeing you're the original author of this PR, I think it would be a reasonable idea to allow you to do a final check on this functionality before we merge. Let's wait for your OAuth to be restored, and once it does let us know if it's all thumbs up for a merge. Staying tuned 👍

@hughandersen
Copy link
Author

@Voyz Ok, no problem. I'll try checking on Sunday afternoon or Monday. @thouseef and @salsasepp seem to have got their connections working (please confirm), so I feel fairly confident that the code is working.

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.

7 participants