exchange_code_for_token

[tifoji]

I am pasting the full URL (http://localhost/?code=xxxx) when I get prompted to enter it after I am starting the workflow by running use_oauth.py but seem to be running into this error. It doesn't provide any other HTTP error codes etc.

One thing I noticed was that the browser that gets launched also gives me an invalid_client error. I think the problem is with the encoding of the url that gets pasted on the browser. So I manually correct it and launch it like

https://auth.tdameritrade.com/auth?response_type=code&redirect_uri=http://localhost&client_id=PRIVATE%40AMER.OAUTHAP

It prompts me for the user/app and after i allow it gives me the full url back which I paste in the terminal

Traceback (most recent call last):
  File "/home/pi/tos/td-ameritrade-api/samples/use_ouath.py", line 16, in <module>
    td_credentials = TdCredentials(
  File "/home/pi/tos/td-ameritrade-api/td/credentials.py", line 69, in __init__
    self.from_workflow()
  File "/home/pi/tos/td-ameritrade-api/td/credentials.py", line 361, in from_workflow
    token_dict = self.exchange_code_for_token(return_refresh_token=True)
  File "/home/pi/tos/td-ameritrade-api/td/credentials.py", line 517, in exchange_code_for_token
    raise requests.HTTPError()
requests.exceptions.HTTPError

[tifoji]

@BillSchumacher

Trying your branch and it results in same error. Any insight is appreciated.

[BillSchumacher]

localhost didn't work for me, I use an https:// enabled domain.

[BillSchumacher]

My use case looks like this.

        td_credentials = TdCredentials(
            client_id=TD_AMERITRADE_CONSUMER_KEY,
            redirect_uri="https://actual_url",
            use_workflow=False
        )
        
        td_credentials.from_token_dict(**creds)
        self.api_client = TdAmeritradeClient(credentials=td_credentials)
        self.streaming_api_client = self.api_client.streaming_api_client(on_message_received=self.message_processor,
                                                                         debug=True)
        self.streaming_services = self.streaming_api_client.services()
        self.streaming_services.quality_of_service(
            qos_level='1'
        )

        self.streaming_services.level_one_quotes(
            symbols=self.symbols,
            fields=LevelOneQuotes.All.value
        )
      

[BillSchumacher]

You need the workflow at least once, and every so often you have to redo it, you should note that the credentials are not saved automatically for you, you have to dump them to a file, database or whatever.

[BillSchumacher]

Additionally, I can only verify that the streaming client works.

[BillSchumacher]

Also, this package does not import properly in it's current state on @areed1192 's repo. It will if you install from mine though.

You can do so by adding this to your requirements.txt file:

py-tda-api @ git+https://github.com/BillSchumacher/td-ameritrade-api.git@374927a5bf9e76434981cf38b0710330e6f071f7

[tifoji]

Thanks Bill. I updated configs with SSL in my webserver (probably not needed), updated the app on the dev site with https://localhost and tried to generate the token for the first time by running use_ouath.py but still get the same error. All packages install successfully and I added your repo ...f7 in requirements , ran it too.

What I find happens in my case is when I run any of the samples so that it triggers the workflow, the browser is launched but I see the 0AUTHP URL very briefly in the browser, but then it redirects immediately to

https://auth.tdameritrade.com/%27https://localhost'?error=invalid_client&error_description=invalid_client

I manually run the URL in browser and it gives me the whole code=xxx string. Then the HTTPError happens.

[BillSchumacher]

Yeah, you got an invalid client error. Probably because you don't have a valid certificate.

[BillSchumacher]

or you didn't setup the consumer key maybe, I'm not sure what the error messages mean.

[tifoji]

No worries. Client certs are ok. Coincidently I checked tda-api which is Alex Golec's project and that one worked for me just fine, including invoking Chrome and then storing the tokens. For me it will come down to this part getting automated as the main criteria for which wrapper to use. I still find great value in this repo and the examples can be extended to the tda-api project as well. Hope Reed gets active again here !

[tifoji]

Also to add, to me it looks like the url encode/decode may need some tweaking. Thanks a lot for your updates and help here.

[BillSchumacher]

No problem, there's a lot of room for improvement here, I've only used this package for a few days or a week now maybe.

I plan on implementing a state machine for the streaming client, eventually, instead of the separate while loops for auth.

The selenium automation is a great idea, something else that was weird was you had to copy and paste the full url into your prompt, not just the code. I had a bit of trouble with the auth stuff at first as well.

This was the library recommended by TDA, and I have audited the code for security concerns already which I am satisfied with so far.

Glad to hear you found something that works for you, I would advise to take a close look at any code that involves your money.

I may completely branch off here as I like to do things a bit different.

[BillSchumacher]

Use of that other library has some serious security concerns, FYI.
https://github.com/alexgolec/tda-api/blob/e791616c27c983292f7ab5acc7b07cbabad25746/tda/contrib/orders.py#L106

    '''
    Returns code that can be executed to construct the given builder, including
    import statements.
    :param builder: :class:`~tda.orders.generic.OrderBuilder` to generate.
    :param var_name: If set, emit code that assigns the builder to a variable
                     with this name.
    '''
    ast = construct_order_ast(builder)

    imports = defaultdict(set)
    lines = []
    ast.render(imports, lines)

    import_lines = []
    for module, names in imports.items():
        line = 'from {} import {}'.format(
                module, ', '.join(names))
        if len(line) > 80:
            line = 'from {} import (\n{}\n)'.format(
                    module, ',\n'.join(names))
        import_lines.append(line)

    if var_name:
        var_prefix = f'{var_name} = '
    else:
        var_prefix = ''

    return autopep8.fix_code(
            '\n'.join(import_lines) + 
            '\n\n' +
            var_prefix +
            '\n'.join(lines))
   

This chunk of code if not carefully used could allow an attacker to completely compromise your system via remote code
execution. There is no good reason for it to be in the package.

Which is used to build orders for some reason, I assume to deliberately create a vulnerability in apps that use the package or plain ignorance, which I very much doubt.

[alexgolec]

Hey there, author of tda-api here. Can you elaborate on your security concerns here? I'm not sure I follow your concerns, but if you see a potential security vulnerability I'd like to close it.

[BillSchumacher]

Sure, so imagine that users of you package allow other people using their software to place orders using their own tda account.

Now imagine that they do not validate user input properly when placing a repeating order for example and they inject python that opens a remote shell or downloads a virus.

Dynamically generating python and executing it should be avoided, there's no reason you can't implement the same logic without creating a gaping hole.

With your background, according to linkedin you should definitely know better.

[alexgolec]

Would you care to point to a location in the library codebase where the library executes the code this method generates?

[BillSchumacher]

Seems this is only used in scripts and tests currently, apologies, no current threat.

Still would not alleviate my concerns, that's dangerous code.

[BillSchumacher]

I don't like importing that file is the issue I suppose.

You might consider creating a separate utils repo with that code in there instead mixed in with everything else.

[BillSchumacher]

Another worry is that some day the package gets updated and bad things start to happen. It's extremely suspect.

Even having that in the test suite is a concern, honestly. There's just too much potential for bad.

[BillSchumacher]

Another thing to consider is how other repos that import your library may use this code.

It's not immediately obvious when it's an external package that functionality exists.

[BillSchumacher]

FYI - alexgolec/tda-api#318
alexgolec/tda-api#317

[alexgolec]

For context for readers, this method is a part of a script which downloads real orders from TD Ameritrade, and emits code that would duplicate those orders. I created this functionality because creating complex orders is difficult, both in terms of constructing them in such a way that TDAmeritrade accepts them, and in terms of actually creating library utilities flexible enough to capture all the possibilities that the platform supports. This solution allows users to create dummy orders through TD Ameritrade that are guaranteed to be correct, and then this utility very easily generates code to duplicate that functionality. This code is not executed, but rather printed to the screen. Users are free to do with it as they please.

In response to @BillSchumacher’s concerns: the only place where this generated code is being executed is in the test suite, in order to verify that it’s syntactically correct. There is no place where this is being executed in the actual shipped library, which means that any concerns around this code are theoretical at best. Still, it does represent an opportunity to improve the tooling around it. Thanks @BillSchumacher for pointing this out, I’ll address this when I get back to my desk.