Return to previous page on openID login

[asdil12]

Ticket: https://progress.opensuse.org/issues/89710

I'm introducing a new dependency Convert::Base32 to encode the return url parameter.
If I don't do this and only do the regular urlencoding that should normally be sufficient,
this error appears:

[debug] [Vsohuwj_cWLv] Routing to controller "OpenQA::Shared::Controller::Session" and action "response"
[debug] Net::OpenID::Consumer: Cache MISS for https://www.opensuse.org/openid/user/dheidler
[debug] Net::OpenID::Consumer: Cache MISS for https://www.opensuse.org/openid/yadis/dheidler.xrds
[debug] Net::OpenID::Consumer: Cache MISS for https://www.opensuse.org/openid/user/dheidler
[debug] Net::OpenID::Consumer: semantic info (https://www.opensuse.org/openid/user/dheidler) =
[debug] Net::OpenID::Consumer: Server is https://www.opensuse.org/openid/
[debug] Net::OpenID::Consumer: verified_identity: assoc_handle: {HMAC-SHA1}{6127a303}{b'it2j7Q=='}
[debug] Net::OpenID::Consumer: handle_assoc: dumb mode: no_cache
[debug] Net::OpenID::Consumer: verified_identity: verifying using HTTP (dumb mode)
[debug] Net::OpenID::Consumer: fail(naive_verify_failed_return) Direct contact invalidated ID provider response.
[error] naive_verify_failed_return: Direct contact invalidated ID provider response.

It seems that no (urlencoded) / or ? (I didn't test for other characters, but I guess it doesn't like the % sign that is used for encoding) must be present in the url to avoid this error.

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏

[okurz]

you need to rebase and please add test coverage for the newly added code

[asdil12]

I rebased but before I add tests, I would like to see if someone has an idea how to deal with the naive_verify_failed_return error without using base32 (the dependency in question is not in tumbleweed at the moment).

[Martchus]

The dependency should at least be in Tumbleweed so you would need to create a SR. Then you can symlink the package from openSUSE:Factory to our Leap devel repos using osc linkpac ….

However, maybe you can also avoid the dependency? So an explicit url_escape instead of encode_base32 doesn't work because the % sign within the encoded string makes problems? Maybe you can still use base64? I suppose the only problematic characters in base64 are =, + and / and you could swap them out by e.g. ~, - and . which likely don't make problems.

[perlpunk]

https://metacpan.org/pod/MIME::Base64 has encode_base64url

[codecov]

Codecov Report

Merging #4151 (8202361) into master (5ffe7a2) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4151      +/-   ##
==========================================
+ Coverage   97.82%   97.89%   +0.07%     
==========================================
  Files         371      371              
  Lines       33272    33290      +18     
==========================================
+ Hits        32548    32589      +41     
+ Misses        724      701      -23     

Impacted Files	Coverage Δ
lib/OpenQA/WebAPI/Auth/OpenID.pm	46.66% <100.00%> (+24.44%)	⬆️
t/03-auth.t	100.00% <100.00%> (ø)
lib/OpenQA/Shared/Controller/Session.pm	77.77% <0.00%> (+24.07%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ffe7a2...8202361. Read the comment docs.

[Martchus]

encode_base64url looks much like my idea of just replacing the problematic characters. So I guess that's how it should be done. Nevertheless it would be good to add tests.

[asdil12]

encode_base64url looks much like my idea of just replacing the problematic characters. So I guess that's how it should be done. Nevertheless it would be good to add tests.

Yes - it seems to work fine but is avoiding the extra dep and is more efficient as well. I will add some tests.

[asdil12]

Looks like OpenID.pm doesn't have much coverage as of now:
https://app.codecov.io/gh/os-autoinst/openQA/blob/master/lib/OpenQA/WebAPI/Auth/OpenID.pm

[Martchus]

It likely requires a lot of mocking to cover this code but the code itself it quite simple so it shouldn't be that much effort after all.

[okurz]

#4156 is how I would simplify the OpenID module a bit but I marked it as not-ready as I don't want to cause conflicts for you and it does not provide a lot of benefit.

[okurz]

OBS fail:

[  601s] 
[  601s]     #   Failed test '302 Found'
[  601s]     #   at ./t/03-auth.t line 54.
[  601s]     #          got: '403'
[  601s]     #     expected: '302'
[  601s] Use of uninitialized value $url in pattern match (m//) at /usr/lib/perl5/vendor_perl/5.26.1/Mojo/URL.pm line 56.
[  601s] Use of uninitialized value $url in pattern match (m//) at /usr/lib/perl5/vendor_perl/5.26.1/Mojo/URL.pm line 56.
[  601s] 
[  601s]     #   Failed test 'return page set'
[  601s]     #   at ./t/03-auth.t line 56.
[  601s]     #          got: undef
[  601s]     #     expected: 'L3Rlc3RzLzQy'
[  601s]     # Looks like you failed 2 tests of 7.
[  601s] 
[  601s] #   Failed test 'OpenID'
[  601s] #   at ./t/03-auth.t line 69.
[  602s] 
[  602s] #   Failed test 'no (unexpected) warnings (via done_testing)'
[  602s] #   at ./t/03-auth.t line 123.
[  602s] # Looks like you failed 2 tests of 8.
[  602s] ./t/03-auth.t ............................................... 

[okurz]

Regarding dependencies, in OBS we use

[   19s] [500/727] cumulate perl-Net-OpenID-Common-1.20-lp152.3.2
[   19s] [501/727] cumulate perl-Selenium-Remote-Driver-1.41-lp152.2.1
[   19s] [502/727] cumulate perl-Perl-Critic-1.130-lp152.3.2
[   19s] [503/727] cumulate perl-DateTime-Locale-1.170000-lp152.3.2
[   19s] [504/727] cumulate perl-Mojo-SQLite-3.007-lp152.2.1
[   19s] [505/727] cumulate perl-Net-OpenID-Consumer-1.18-lp152.3.2

and in https://github.com/os-autoinst/openQA/blob/master/.circleci/ci-packages.txt we have the same versions Net-OpenID-Common-1.20 and Net-OpenID-Consumer-1.18 so at least that is the same.

[asdil12]

My idea that the obs issue could be caused by a lack of internet access became less likely when I disconnected my local machine from the wifi and successfully ran the testsuite.

[asdil12]

Looking at the source of Net:OpenID:Consumer->claimed_identity() it still looks like as if there was some kind of discovery involved. Maybe there is some caching involved that complicated things.

I try to mock this interaction.
According to the OpenID Connect spec, the Consumer<-->IdentityProvider interaction should only happen after the user returned from login.

[kalikiana]

unresolvable: nothing provides libIlmImf-2_5.so.25()(64bit) needed by libopencv_imgcodecs4_5

FYI this is also hitting poo#97700