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

When have multi clients, The API client's Oauth2Token keeps the same #99

Open
ezioruan opened this issue Sep 23, 2022 · 12 comments
Open
Assignees

Comments

@ezioruan
Copy link
Contributor

ezioruan commented Sep 23, 2022

I have an app that connects to many client_ids, and I found their's client_id and client_secret keep the same even though I put different config,
here are some test codes:



from xero_python.api_client import ApiClient
from xero_python.api_client.configuration import Configuration
from xero_python.api_client.oauth2 import OAuth2Token


api_client1 = ApiClient(
    Configuration(
        debug=True,
        oauth2_token=OAuth2Token(
            client_id='id1', client_secret='secret1'
        ),
    ),
    pool_threads=1,
)


api_client2 = ApiClient(
    Configuration(
        debug=True,
        oauth2_token=OAuth2Token(
            client_id='id2', client_secret='secret2'
        ),
    ),
    pool_threads=1,
)


print('client1', api_client1.configuration.oauth2_token.__dict__)
print('client2', api_client2.configuration.oauth2_token.__dict__)

And the output is :

client1 {'client_id': 'id1', 'client_secret': 'secret1', 'expiration_buffer': 60, 'token_type': 'Bearer', 'access_token': None, 'id_token': None, 'refresh_token': None, 'scope': None, 'expires_at': None, 'expires_in': None}

client2 {'client_id': 'id1', 'client_secret': 'secret1', 'expiration_buffer': 60, 'token_type': 'Bearer', 'access_token': None, 'id_token': None, 'refresh_token': None, 'scope': None, 'expires_at': None, 'expires_in': None}

You can see they are the same client_id and client_secrets

I am wondering if it's the correct behavior or is a bug,

I found the root cause here https://github.com/XeroAPI/xero-python/blob/master/xero_python/api_client/configuration.py#L41.

class Configuration(metaclass=TypeWithDefault):

The Configuration's metadata makes him always copy attributes from the same instance.
What's the purpose of doing this?

@ezioruan ezioruan changed the title The API cli When have multi clients, The API client's Oauth2Token keeps the same Sep 23, 2022
@RettBehrens
Copy link
Contributor

Hmmm. Could you detail your use case? What's the reasoning for swapping between ClientID and ClientSecret credentials?

@RettBehrens RettBehrens self-assigned this Sep 23, 2022
@ezioruan
Copy link
Contributor Author

@RettBehrens We set up each app for each billing entity because some business logic there has different scopes and we need to separate them. And run some corn job base on them. for now I have to assign the OauthToken again to avoid this issues

 oauth2_token = OAuth2Token(
        client_id=client_id,
        client_secret=client_secret,
    )
    configuration = Configuration(
        debug=True,
        oauth2_token=oauth2_token,
    )

    api_client = ApiClient(
        configuration=configuration,
        pool_threads=1,
    )
    api_client.configuration.oauth2_token = oauth2_token

I am not sure if other languages' SDK also have this behavior.

@ezioruan
Copy link
Contributor Author

@RettBehrens Test from nodejs sdk. it works as excepted. differences API clients have different config. here's the test code

const { XeroClient } = require('xero-node');


const xero1 = new XeroClient({
  clientId: 'clientId1', // required
  clientSecret: 'clientSecret1', // required
  redirectUris: [`http://localhost/callback`], // not used for client_credentials auth flow
  grantType: 'client_credentials', // only used for client_credentials auth flow
  scopes: 'openid profile email accounting.transactions offline_access'.split(" "), // not used for client_credentials auth flow
  state: 'returnPage=my-sweet-dashboard', // custom params (optional), not used for client_credentials auth flow
  httpTimeout: 3000 // ms (optional)
});

const xero2 = new XeroClient({
  clientId: 'clientId2', // required
  clientSecret: 'clientSecret2', // required
  redirectUris: [`http://localhost/callback`], // not used for client_credentials auth flow
  grantType: 'client_credentials', // only used for client_credentials auth flow
  scopes: 'openid profile email accounting.transactions offline_access'.split(" "), // not used for client_credentials auth flow
  state: 'returnPage=my-sweet-dashboard', // custom params (optional), not used for client_credentials auth flow
  httpTimeout: 3000 // ms (optional)
});


console.log("xero1",xero1.config)
console.log("xero2",xero2.config)

output:

xero1 {
  clientId: 'clientId1',
  clientSecret: 'clientSecret1',
  redirectUris: [ 'http://localhost/callback' ],
  grantType: 'client_credentials',
  scopes: [
    'openid',
    'profile',
    'email',
    'accounting.transactions',
    'offline_access'
  ],
  state: 'returnPage=my-sweet-dashboard',
  httpTimeout: 3000
}
xero2 {
  clientId: 'clientId2',
  clientSecret: 'clientSecret2',
  redirectUris: [ 'http://localhost/callback' ],
  grantType: 'client_credentials',
  scopes: [
    'openid',
    'profile',
    'email',
    'accounting.transactions',
    'offline_access'
  ],
  state: 'returnPage=my-sweet-dashboard',
  httpTimeout: 3000
}

@RettBehrens
Copy link
Contributor

RettBehrens commented Sep 26, 2022

Hi @ezioruan what you're describing is against Xero Developer Platform Ts&Cs. Specifically section 7b Be a Good Citizen to our Ecosystem:

You share Xero and our Developer Platform with your fellow developers and you should write Your Application as you’d want others to write theirs. You agree not to use, nor permit any third party to use, the Developer Platform to:

  • Create multiple versions of Your Applications that access the Developer Platform for the same or similar usages (e.g. creating customer-specific versions of Your Applications);

Suggested resolution - use one set of ClientID and ClientSecret credentials, adjusting what scopes are requested during auth/consent

@ezioruan
Copy link
Contributor Author

@RettBehrens Thanks for your reply, so it's the right behavior. but why node js client can have a differences client_id for each api_client?

@RettBehrens
Copy link
Contributor

@ezioruan configuration.py in Xero-Python SDK is based on a fork of OpenAPITools openapi-generator python module.

Looking at the source file from openapi-generator, I can see there is no TypeWithDefault class there, so it's presumably something the original author added.

In the Xero-Node SDK, while most of the files generated, XeroClient.ts was home-rolled rather than generated, so it's possible this restriction should have been added but was missed.

I will need to investigate further and consult with the team to determine which is the correct / expected behavior.

@ezioruan
Copy link
Contributor Author

Looks good. Thanks for your reply. I don't know how to use the generate tools. Could you take a look to see if the newly generated files are correct (without TypeWithDefault )

@pernofence
Copy link

We are having the same issue. We are currently using two client objects, one is using a private connection auth, and the other is using code flow auth. When using both instances in the same process it fails because the client id/secret gets mixed up.

@RettBehrens
Copy link
Contributor

An update on this topic. I believe I've found an issue identifying the same problem with OpenAPITools openapi-generator. Following the thread, it seems TypeWithDefault was added at some time and ultimately removed - our fork of the generator must have occurred after it was added but before it was removed. Going to take some time but I'll create a ticket to refactor the configuration so it doesn't use TypeWithDefault.

@ezioruan
Copy link
Contributor Author

Looks like the bug was reported years ago but still have problems, hope we can solve this .

If there's some document about how to generate those codes I think it would be a big help

@tamhv
Copy link

tamhv commented Mar 20, 2023

if we buy 2 custom connections for 2 organizations and use them in a python app, then we would end up here, the 2nd client_id can't be used

@pernofence
Copy link

This workaround seems to work for me:

from xero_python.api_client import Configuration as BaseConfiguration
from xero_python.api_client.configuration import TypeWithDefault

class Workaround(TypeWithDefault):
    def __call__(cls, *args, **kwargs):
        return super(TypeWithDefault, cls).__call__(*args, **kwargs)

class Configuration(BaseConfiguration, metaclass=Workaround):
    pass

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

No branches or pull requests

4 participants