CI random failure: TestExportUserGPGKeys #22105
Comments
|
Happening for us about ~10% of the time. Happy to help in any way, just tag me 👍 |
|
IMO it's caused by incorrect CSRF token.
ps: there are other redirections in
|
|
Perhaps we should do: func getTokenForLoggedInUser(t testing.TB, session *TestSession) string {
t.Helper()
var token string
for i := 0; i < 3 && token == ""; i++ {
req := NewRequest(t, "GET", "/user/settings/applications")
resp := session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
req = NewRequestWithValues(t, "POST", "/user/settings/applications", map[string]string{
"_csrf": doc.GetCSRF(),
"name": fmt.Sprintf("api-testing-token-%d", atomic.AddInt64(&tokenCounter, 1)),
})
session.MakeRequest(t, req, http.StatusSeeOther)
req = NewRequest(t, "GET", "/user/settings/applications")
resp = session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
token = htmlDoc.doc.Find(".ui.info p").Text()
}
assert.NotEmpty(t, token)
return token
} |
|
Of course this doesn't explain why the CSRF would ever be incorrect etc. |
|
ps: CSRF is just my guess (the best guess in my mind), I haven't looked into it. There are other redirections in Also it could output the CSRF token to log to see whether it is incorrect or doesn't exist (can not be loaded from HTML response). That's really strange that only this test case fails. |
|
I don't understand it either - there's a bit of caching that happens related to the users in the test cases but that shouldn't really be a problem. |
There are repeated failures with this test which appear related to failures in getTokenForLoggedInUser. It is difficult to further evaluate the cause of these failures as we do not get given further information. This PR will attempt to fix this. First it adds some extra logging and it uses the csrf cookie primarily for the csrf value. If the problem does not occur again with those changes we could merge, assume that it is fixed and hope that if it occurs in future the additional logging will be helpful. If not I will add more changes in attempt to fix. Fix go-gitea#22105 Signed-off-by: Andrew Thornton <art27@cantab.net>
|
I think TestAPITeam is also affected by the same underlying problem too. |
There are repeated failures with this test which appear related to failures in getTokenForLoggedInUser. It is difficult to further evaluate the cause of these failures as we do not get given further information. This PR will attempt to fix this. First it adds some extra logging and it uses the csrf cookie primarily for the csrf value. If the problem does not occur again with those changes we could merge, assume that it is fixed and hope that if it occurs in future the additional logging will be helpful. If not I will add more changes in attempt to fix. Fix #22105 Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: John Olheiser <john.olheiser@gmail.com> Co-authored-by: techknowlogick <matti@mdranta.net> Co-authored-by: delvh <dev.lh@web.de>
|
Tests failed again: https://drone.gitea.io/go-gitea/gitea/64747/2/14 Maybe it's related with the csrf token expired time, it's 5 seconds in non-prod mode. ref: 84ceaa9#diff-fa07e296d614e796de67f7ae9268a0660b89c09b6e5f6373b4ca7bf76a20dec7R669-R671 |
No, the code says: The CsrfTokenRegenerationInterval is only for token generation. When you do validate, it uses the CsrfTokenTimeout which is always 24h. |
|
At 84ceaa9#diff-c45c8effe9ddcfe4e6c08790d2d5841096f6a7d390bcfd23d781ee6bc05474c3R186, it will regenerate the token. |
|
Then? And think about another case, when you run Gitea in dev mode, do you ever see a CSRF error when you stay in a page with form for more than 5 seconds then submit the form? |
|
I sent #22199 to exclude the possible session cache problem. |
|
The additional logging provided in #22159 definitely indicates that the issue is an out of date csrf token. I can only think it is something to do with the cached session but I don't fully understand it as we should be getting a csrf token from the preceding GETs. Excluding the cached session seems like a reasonable attempt at a solution. |
|
If the CSRF token was ever printed, then we can parse it to see what happened.
I do not think a token would expire in such case, because the expiration timeout is 24h. |
|
Perhaps it's something to do with the cookie jar? I don't know if we update the "session" cookie jar on each request - hopefully we do - but could it be that there's an old csrf cookie being passed in addition to the _csrf field on the post? I guess @wxiaoguang is right and we need to add even more logging here. |
|
For anyone struggling with this, we've had good success with the following temporary change. Naturally this means the test will not be run until this change is reverted. diff --git a/integrations/user_test.go b/integrations/user_test.go
index 6a3d30472d..8a40056561 100644
--- a/integrations/user_test.go
+++ b/integrations/user_test.go
@@ -6,6 +6,7 @@ package integrations
import (
"net/http"
+ "os"
"testing"
issues_model "code.gitea.io/gitea/models/issues"
@@ -139,6 +140,10 @@ func TestRenameReservedUsername(t *testing.T) {
}
func TestExportUserGPGKeys(t *testing.T) {
+ if os.Getenv("DRONE") != "" {
+ t.Skip("FIXME: Skipping this test in CI until https://github.com/go-gitea/gitea/issues/22105 is resolved")
+ }
+
defer prepareTestEnv(t)()
// Export empty key list
testExportUserGPGKeys(t, "user1", `-----BEGIN PGP PUBLIC KEY BLOCK----- |
|
Looks like this testing random error disappeared recently. |
|
Have not seen this in a while now, let's close. |
The text was updated successfully, but these errors were encountered: