exchange_code_for_token about td-ameritrade-api HOT 22 OPEN

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.

from td-ameritrade-api.

@BillSchumacher

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

from td-ameritrade-api.

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

from td-ameritrade-api.

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
        )
      

from td-ameritrade-api.

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.

from td-ameritrade-api.

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

from td-ameritrade-api.

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

from td-ameritrade-api.

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.

from td-ameritrade-api.

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

from td-ameritrade-api.

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

from td-ameritrade-api.

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 !

from td-ameritrade-api.

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.

from td-ameritrade-api.

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.

from td-ameritrade-api.

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.

from td-ameritrade-api.

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.

from td-ameritrade-api.

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.

from td-ameritrade-api.

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

from td-ameritrade-api.

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

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

from td-ameritrade-api.

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.

from td-ameritrade-api.

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.

from td-ameritrade-api.

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.

from td-ameritrade-api.

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

from td-ameritrade-api.