cloudstack
cloudstack copied to clipboard
apache
Fixup response code on incorrect credentials
Description
This PR fixes https://github.com/apache/cloudstack/issues/8662
The issue got introduced in 4.19. When we try to login, we check login credentials using different types of authenticators. While verifying in case of wrong password with oauth2 authenticator and there are no providers setup for it, it throws a CloudRuntime Exception. Because the error isn't caught, we respond with 200 status code but with empty response. This results in UI getting stuck. This PR fixes this by skipping the authentication check if the secret code is null.
While logging in, if the user doesn't exist it fails with a valid error
If the user exists, but the password is wrong then loading starts on top and the UI gets stuck. I had to clean up my local storage to make the browser work again.
In order to reproduce, use username as "admin" or any other user which exists. For password, enter anything except the password.
Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] build/CI
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
Codecov Report
Attention: Patch coverage is 21.42857% with 11 lines in your changes are missing coverage. Please review.
Project coverage is 14.95%. Comparing base (
c07953c) to head (160aac4). Report is 3 commits behind head on 4.19.
Additional details and impacted files
@@ Coverage Diff @@
## 4.19 #8671 +/- ##
============================================
+ Coverage 4.30% 14.95% +10.65%
- Complexity 0 10985 +10985
============================================
Files 363 5373 +5010
Lines 29296 469164 +439868
Branches 5115 58928 +53813
============================================
+ Hits 1260 70171 +68911
- Misses 27894 391223 +363329
- Partials 142 7770 +7628
| Flag | Coverage Δ | |
|---|---|---|
| uitests | 4.30% <ø> (ø) |
|
| unittests | 15.66% <21.42%> (?) |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 8690
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8695
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8705
@vishesh92 a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
code LGTM. @vishesh92 can you please quickly verify the same when oauth is enabled.
Same issue was happening even if oauth was enabled because the provider name is not there in the request params. It's resolved after this change.
code LGTM. @vishesh92 can you please quickly verify the same when oauth is enabled.
Same issue was happening even if oauth was enabled because the provider name is not there in the request params. It's resolved after this change.
thanks @vishesh92
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8712
[SF] Trillian test result (tid-9284) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 49654 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8671-t9284-kvm-centos7.zip Smoke tests completed. 129 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below:
| Test | Result | Time (s) | Test File |
|---|
@vishesh92 a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
[SF] Trillian test result (tid-9306) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 42343 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8671-t9306-kvm-centos7.zip Smoke tests completed. 129 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below:
| Test | Result | Time (s) | Test File |
|---|
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 9055
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 9059
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 9062