dylanpulver opened a new pull request, #45977:
URL: https://github.com/apache/superset/pull/45977
### SUMMARY
Four tests in `TestGuestTokens` ended with a bare
`self.assertRaisesRegex(...)`. That builds a
context manager and discards it, so nothing is checked. Replacing all four
with an exception nothing
can raise leaves the eight guest-token cases passing, which is what shows
they were dead. Each line
carried `# noqa: PT027`, the rule that flags a unittest-style raises outside
a `with` block.
They are also describing behaviour the code no longer has.
`get_guest_user_from_request` does raise `ValueError("Guest token does not
contain a user claim")`
and friends, but wraps the lot in `except Exception`, logs `Invalid guest
token`, and returns
`None`. Three of the four tests already assert `guest_user is None`, so for
those the dead line is
simply removed.
`test_get_guest_user_no_resource` is the one that needed more. It asserted
nothing at all, and could
not have asserted the rejection it is named for: it passed `resources=[]`,
and an empty list is not
a missing claim, so the token was accepted and a `GuestUser` returned. It
now builds the token with
no `resources` claim, the way `test_get_guest_user_not_guest_type` builds
its own, which reaches the
path the test is named for.
Happy to drop that last change and leave the test asserting current
behaviour instead, if you would
rather keep the diff to removals.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable.
### TESTING INSTRUCTIONS
```
pytest tests/integration_tests/security_tests.py -k test_get_guest_user
```
- unchanged: 8 passed
- unchanged, with all four expectations swapped for `ZeroDivisionError`: 8
passed, so none of them ran
- with this change: 8 passed
- with `assert guest_user is None` negated in
`test_get_guest_user_no_resource`: 1 failed
- with the `resources` claim added back to that crafted token: 1 failed, so
omitting it is what
drives the rejection
The rest of `security_tests.py` gives an identical `43 failed, 40 passed, 2
skipped, 3 errors`
before and after this change in my environment, which has no seeded example
data. Every
`TestGuestTokens` case passes either way.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
