Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show warning in case the server cuts out auth header #221

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

VicDeo
Copy link
Member

@VicDeo VicDeo commented Sep 20, 2019

While on Admin Settings page end an ajax request and show a warning if the Authorization header is stripped by the webserver

@VicDeo VicDeo added this to the development milestone Sep 20, 2019
@VicDeo VicDeo self-assigned this Sep 20, 2019
@CLAassistant
Copy link

CLAassistant commented Sep 20, 2019

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Sep 20, 2019

Codecov Report

Merging #221 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #221      +/-   ##
============================================
- Coverage     68.77%   68.72%   -0.05%     
- Complexity      228      229       +1     
============================================
  Files            35       35              
  Lines           903      905       +2     
============================================
+ Hits            621      622       +1     
- Misses          282      283       +1
Impacted Files Coverage Δ Complexity Δ
appinfo/routes.php 100% <ø> (ø) 0 <0> (ø) ⬇️
lib/Controller/SettingsController.php 98.57% <100%> (+0.04%) 12 <1> (+1) ⬆️
lib/Db/Client.php 91.66% <0%> (-8.34%) 4% <0%> (ø)

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 7196bf6...ef61fff. Read the comment docs.

@jvillafanez
Copy link
Member

Code looks fine.
Could we plan it a bit more generic to include other tests there? Otherwise we could keep adding more endpoints for testing, which doesn't look good. We should try to minimize these "test" endpoints because they don't right from an API's point of view.

@VicDeo
Copy link
Member Author

VicDeo commented Sep 20, 2019

@jvillafanez could you be a bit more specific on what I need to improve. You want me to rename testAuth into just test, correct?

As for now I just added a clue to the issue I had with a phoenix so nobody else will stuck at the same point.

js/settings.js Outdated Show resolved Hide resolved
@VicDeo VicDeo force-pushed the check-server branch 3 times, most recently from 6b0e450 to d931388 Compare September 20, 2019 13:55
@jvillafanez
Copy link
Member

You want me to rename testAuth into just test, correct?

That's partially yes. I'd prefer to have just one endpoint to perform sanity / health checks instead of end up with multiple endpoints for this matter.
I think this is fixed with the latest changes. We could easily add new information in the /test endpoint if needed.

@mmattel
Copy link

mmattel commented Jan 21, 2020

Ping, any update on this?

@VicDeo VicDeo force-pushed the check-server branch 4 times, most recently from 9755dd5 to ef61fff Compare January 21, 2020 08:30
@VicDeo VicDeo force-pushed the check-server branch 2 times, most recently from 0f53615 to 4509594 Compare February 25, 2021 12:03
@GeraldLeikam GeraldLeikam modified the milestones: development, QA Sep 14, 2021
@mmattel
Copy link

mmattel commented Oct 11, 2021

any update?

@jvillafanez
Copy link
Member

Other than rebasing to update the code, I think this is ready. Anything left to do @VicDeo

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline webUIOauth2-latest-chrome-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/oauth2/1701/15/1

@mmattel
Copy link

mmattel commented Oct 12, 2021

@phil-davis mind to have a look why the build was killed...?

js/settings.js Outdated Show resolved Hide resolved
@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline webUIOauth2-latest-chrome-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/oauth2/1703/15/1

@mmattel
Copy link

mmattel commented Oct 19, 2021

Rebased

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline webUIOauth2-latest-chrome-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/oauth2/1710/15/1

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline webUIOauth2-latest-chrome-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/oauth2/1740/15/1

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline webUIOauth2-master-chrome-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/oauth2/1740/14/1

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

Successfully merging this pull request may close these issues.

7 participants