Skip to content

Conversation

@juliusknorr
Copy link
Member

This used to work for most cases in the far past as jquery adds this on requests (https://github.com/jquery/jquery/blob/48cc402a917d6011c7d3e75f779f11ef91b474fb/src/ajax/xhr.js#L43-L50)

nextcloud-libraries/nextcloud-axios#637 adds this to our axios wrapper but this PR handles cases where apps or code parts are not using that.

Server has logic in place to return a dummyauth response header in case this one is set:
dfc3536

This helps to handle requests that happen in the web UI after authentication was revoked (e.g. by disabling a user) more gracefully, otherwise a request would lead to the browser showing a basic auth dialog.

Steps to reproduce:

  • Login as a user
  • Disable the user through occ
  • Wait until the notifications app sends another request

Checklist

@juliusknorr juliusknorr force-pushed the bugfix/noid/x-requested-with branch from 153d971 to b83af90 Compare August 17, 2023 12:48
@juliusknorr juliusknorr requested review from a team, Pytal, artonge, julien-nc and skjnldsv and removed request for a team August 17, 2023 12:48
@juliusknorr juliusknorr added 3. to review Waiting for reviews bug labels Aug 17, 2023
@juliusknorr juliusknorr added this to the Nextcloud 28 milestone Aug 17, 2023
@juliusknorr juliusknorr marked this pull request as ready for review August 18, 2023 08:01
@juliusknorr juliusknorr force-pushed the bugfix/noid/x-requested-with branch from b83af90 to bd12464 Compare August 18, 2023 08:01
@artonge
Copy link
Contributor

artonge commented Aug 18, 2023

In which cases do we not use axios?

@juliusknorr
Copy link
Member Author

Old code or apps that might not use our libraries, rare cases but might still happen

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 18, 2023
@skjnldsv skjnldsv force-pushed the bugfix/noid/x-requested-with branch from bd12464 to 6d79339 Compare August 18, 2023 18:12
@skjnldsv
Copy link
Member

/compile amend /

…h dialogs

Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: nextcloud-command <[email protected]>
@nextcloud-command nextcloud-command force-pushed the bugfix/noid/x-requested-with branch from 6d79339 to 0904c84 Compare August 18, 2023 18:22
@skjnldsv skjnldsv merged commit af87086 into master Aug 18, 2023
@skjnldsv skjnldsv deleted the bugfix/noid/x-requested-with branch August 18, 2023 21:45
@juliusknorr
Copy link
Member Author

/backport to stable27

@juliusknorr
Copy link
Member Author

/backport to stable26

@juliusknorr
Copy link
Member Author

/backport to stable25

@juliusknorr
Copy link
Member Author

/backport to stable24

@backportbot-nextcloud
Copy link

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@backportbot-nextcloud
Copy link

The backport to stable24 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable24
git pull origin stable24

# Create the new backport branch
git checkout -b fix/foo-stable24

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable24

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@backportbot-nextcloud
Copy link

The backport to stable26 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable26
git pull origin stable26

# Create the new backport branch
git checkout -b fix/foo-stable26

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable26

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@backportbot-nextcloud
Copy link

The backport to stable25 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable25
git pull origin stable25

# Create the new backport branch
git checkout -b fix/foo-stable25

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable25

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

Comment on lines +46 to +50
if (init.headers instanceof Headers && !init.headers.has('X-Requested-With')) {
init.headers.append('X-Requested-With', 'XMLHttpRequest')
} else if (init.headers instanceof Object && !init.headers['X-Requested-With']) {
init.headers['X-Requested-With'] = 'XMLHttpRequest'
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juliushaertl a small find as of today:
image

If headers is an instance of Headers and already have X-Requested-With, then the second else if will still be true.
Luckily, ['X-Requested-With'] on a Headers object is ignored by fetch 🙈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants