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

Validate token audience before running gather #114

Closed
wants to merge 1 commit into from

Conversation

rgmz
Copy link
Contributor

@rgmz rgmz commented Jan 23, 2025

This checks that the provided token is for graph.windows.net before running roadrecon gather, as mentioned in #111.

Add a sanity check in run that the token is for graph.windows.net

As with #113, this could be prettier but I tried to keep the changes minimal:

$ roadrecon gather
Traceback (most recent call last):
  File "/home/user/code/ROADtools/.venv/bin/roadrecon", line 8, in <module>
    sys.exit(main())
             ~~~~^^
  File "/home/user/code/ROADtools/roadrecon/roadtools/roadrecon/main.py", line 125, in main
    gathermain(args)
    ~~~~~~~~~~^^^^^^
  File "/home/user/code/ROADtools/roadrecon/roadtools/roadrecon/gather.py", line 740, in main
    raise ValueError("token audience must be 'https://graph.windows.net', not '%s'" % aud)
ValueError: token audience must be 'https://graph.windows.net', not 'https://graph.microsft.com'

@rgmz rgmz force-pushed the feat/gather-validate-token branch from 473d33c to 8efcb4f Compare January 23, 2025 13:13
@rgmz rgmz force-pushed the feat/gather-validate-token branch from 8efcb4f to cb61b5c Compare January 23, 2025 13:14
@dirkjanm
Copy link
Owner

hey, appreciate the PR, however there's already some token validation in roadtx that we can re-use that uses the functionality from roadlib. we also need to take into account multiple valid audiences, as added in 5f87886

@dirkjanm dirkjanm closed this Jan 23, 2025
@rgmz rgmz deleted the feat/gather-validate-token branch January 23, 2025 18:02
@rgmz
Copy link
Contributor Author

rgmz commented Jan 23, 2025

No worries, that's definitely a much better implementation!

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.

2 participants