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

return expires_on from get-access-token #27410

Closed
wants to merge 1 commit into from

Conversation

cataggar
Copy link
Member

@cataggar cataggar commented Sep 17, 2023

This is a fix for #19700. @jiasli made the code. It works. It will let us fix Azure/azure-sdk-for-rust#1371 .

  "expiresOn": "2023-09-17 20:52:34.000000",
  "expires_on": 1694976754,

If you want to rename it to expiresOnEpoch, either works for us.

Related command
az account get-access-token

Description
This returns the expire_on.

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change
[Component Name 2] az command b: Add some customer-facing feature


This checklist is used to make sure that common guidelines for a pull request are followed.

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Sep 17, 2023

❌AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.10
️✔️3.9
️✔️ams
️✔️latest
️✔️3.10
️✔️3.9
️✔️apim
️✔️latest
️✔️3.10
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.10
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.10
️✔️3.9
️✔️aro
️✔️latest
️✔️3.10
️✔️3.9
️✔️backup
️✔️latest
️✔️3.10
️✔️3.9
️✔️batch
️✔️latest
️✔️3.10
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.10
️✔️3.9
️✔️billing
️✔️latest
️✔️3.10
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.10
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.10
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.10
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.10
️✔️3.9
️✔️config
️✔️latest
️✔️3.10
️✔️3.9
️✔️configure
️✔️latest
️✔️3.10
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.10
️✔️3.9
️✔️container
️✔️latest
️✔️3.10
️✔️3.9
🔄containerapp
🔄latest
🔄3.10
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.10
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️dla
️✔️latest
️✔️3.10
️✔️3.9
️✔️dls
️✔️latest
️✔️3.10
️✔️3.9
️✔️dms
️✔️latest
️✔️3.10
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.10
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.10
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.10
️✔️3.9
️✔️find
️✔️latest
️✔️3.10
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.10
️✔️3.9
️✔️identity
️✔️latest
️✔️3.10
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️kusto
️✔️latest
️✔️3.10
️✔️3.9
️✔️lab
️✔️latest
️✔️3.10
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.10
️✔️3.9
️✔️maps
️✔️latest
️✔️3.10
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.10
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.10
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.10
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.10
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.10
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.10
️✔️3.9
❌profile
❌latest
❌3.10
Type Test Case Error Message Line
Failed test_get_raw_token self = <azure.cli.command_modules.profile.tests.latest.test_profile_custom.ProfileCommandTest testMethod=test_get_raw_token>
get_raw_token_mock = <function get_raw_token at 0x7feb3c208310>

    @mock.patch('azure.cli.core._profile.Profile.get_raw_token', autospec=True)
    def test_get_raw_token(self, get_raw_token_mock):
        cmd = mock.MagicMock()
        cmd.cli_ctx = DummyCli()
    
        get_raw_token_mock.return_value = (['bearer', 'token123', {'expiresOn': '2100-01-01'}], 'sub123', 'tenant123')
    
        result = get_access_token(cmd)
    
        # assert
        get_raw_token_mock.assert_called_with(mock.ANY, None, None, None, None)
        expected_result = {
            'tokenType': 'bearer',
            'accessToken': 'token123',
            'expiresOn': '2100-01-01',
            'subscription': 'sub123',
            'tenant': 'tenant123'
        }
>       self.assertEqual(result, expected_result)
E       AssertionError: {'tok[49 chars]pires_on': None, 'expiresOn': '2100-01-01', 't[41 chars]123'} != {'tok[49 chars]piresOn': '2100-01-01', 'subscription': 'sub12[21 chars]123'}
E         {'accessToken': 'token123',
E          'expiresOn': '2100-01-01',
E       -  'expires_on': None,
E          'subscription': 'sub123',
E          'tenant': 'tenant123',
E          'tokenType': 'bearer'}

src/azure-cli/azure/cli/command_modules/profile/tests/latest/test_profile_custom.py:53: AssertionError
azure/cli/command_modules/profile/tests/latest/test_profile_custom.py:34
❌3.9
Type Test Case Error Message Line
Failed test_get_raw_token self = <azure.cli.command_modules.profile.tests.latest.test_profile_custom.ProfileCommandTest testMethod=test_get_raw_token>
get_raw_token_mock = <function get_raw_token at 0x7f0029257b80>

    @mock.patch('azure.cli.core._profile.Profile.get_raw_token', autospec=True)
    def test_get_raw_token(self, get_raw_token_mock):
        cmd = mock.MagicMock()
        cmd.cli_ctx = DummyCli()
    
        get_raw_token_mock.return_value = (['bearer', 'token123', {'expiresOn': '2100-01-01'}], 'sub123', 'tenant123')
    
        result = get_access_token(cmd)
    
        # assert
        get_raw_token_mock.assert_called_with(mock.ANY, None, None, None, None)
        expected_result = {
            'tokenType': 'bearer',
            'accessToken': 'token123',
            'expiresOn': '2100-01-01',
            'subscription': 'sub123',
            'tenant': 'tenant123'
        }
>       self.assertEqual(result, expected_result)
E       AssertionError: {'tok[49 chars]pires_on': None, 'expiresOn': '2100-01-01', 't[41 chars]123'} != {'tok[49 chars]piresOn': '2100-01-01', 'subscription': 'sub12[21 chars]123'}
E         {'accessToken': 'token123',
E          'expiresOn': '2100-01-01',
E       -  'expires_on': None,
E          'subscription': 'sub123',
E          'tenant': 'tenant123',
E          'tokenType': 'bearer'}

src/azure-cli/azure/cli/command_modules/profile/tests/latest/test_profile_custom.py:53: AssertionError
azure/cli/command_modules/profile/tests/latest/test_profile_custom.py:34
️✔️rdbms
️✔️latest
️✔️3.10
️✔️3.9
️✔️redis
️✔️latest
️✔️3.10
️✔️3.9
️✔️relay
️✔️latest
️✔️3.10
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️role
️✔️latest
️✔️3.10
️✔️3.9
️✔️search
️✔️latest
️✔️3.10
️✔️3.9
️✔️security
️✔️latest
️✔️3.10
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.10
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.10
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.10
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.10
️✔️3.9
️✔️sql
️✔️latest
️✔️3.10
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.10
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.10
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️util
️✔️latest
️✔️3.10
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9

@azure-client-tools-bot-prd
Copy link

Hi @cataggar,
Since the current milestone time is less than 7 days, this pr will be reviewed in the next milestone.

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Sep 17, 2023

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@yonzhan
Copy link
Collaborator

yonzhan commented Sep 17, 2023

Thank you for your contribution! We will review the pull request and get back to you soon.

@microsoft-github-policy-service microsoft-github-policy-service bot added the ARM az resource/group/lock/tag/deployment/policy/managementapp/account management-group label Sep 17, 2023
@jiasli
Copy link
Member

jiasli commented Sep 18, 2023

Thanks for the contribution. I will discuss with our PMs regarding this topic.

@jiasli
Copy link
Member

jiasli commented Sep 21, 2023

@cataggar, do you prefer an integer expires_on or string expires_on?

  • "expires_on": 1694976754
  • "expires_on": "1694976754"

@cataggar
Copy link
Member Author

@jiasli, I think a numeric type is better than a string. A uint32 would be good until 2106-02-07 06:28:15.

@jiasli
Copy link
Member

jiasli commented Sep 27, 2023

As there are more things to do besides simply uncommenting this line, I will do these tasks in another PR #27476.

@jiasli jiasli closed this Sep 27, 2023
@cataggar cataggar deleted the expires_on branch September 27, 2023 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARM az resource/group/lock/tag/deployment/policy/managementapp/account management-group Auto-Assign Auto assign by bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AzureCliCredential parses an incorrect token expiry time in multi-threaded Unix applications
4 participants