Skip to content

Explicitly allow creation of MC without auth #249

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

Merged
merged 12 commits into from
Jul 16, 2025

Conversation

JanCaha
Copy link
Contributor

@JanCaha JanCaha commented Jul 10, 2025

Use @check_token on functions that require authorization and not on all requests towards server. That allows some requests like /config and functions related to it to be usable without user being logged.

Simplify function login() not to use custom request logic, but utilize post() function that is part of MerginClient.

Add tests cases.

@coveralls
Copy link

coveralls commented Jul 10, 2025

Pull Request Test Coverage Report for Build 16317812920

Details

  • 52 of 53 (98.11%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 80.334%

Changes Missing Coverage Covered Lines Changed/Added Lines %
mergin/client.py 31 32 96.88%
Totals Coverage Status
Change from base Build 16073839998: 0.2%
Covered Lines: 3468
Relevant Lines: 4317

💛 - Coveralls

@tomasMizera
Copy link
Contributor

@JanCaha do we need to release this for the upcoming qgis-plugin release?

@JanCaha
Copy link
Contributor Author

JanCaha commented Jul 10, 2025

If we want to solve the SSO auth properly then quite likely yes. I will discuss it tommorow on standup with Stefanos

Copy link
Contributor

@MarcelGeo MarcelGeo left a comment

Choose a reason for hiding this comment

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

Just wondering if this has_auth will be valid after some actions.

I thinks _auth_params is mos important thing, as there is stored username and password.

we can ask also @wonder-sk for 👁️

@MarcelGeo MarcelGeo requested review from uclaros and wonder-sk July 11, 2025 09:25
Copy link
Contributor

@uclaros uclaros left a comment

Choose a reason for hiding this comment

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

Good job!
How about removing the leading _ now that check_token is used everywhere?

@JanCaha
Copy link
Contributor Author

JanCaha commented Jul 14, 2025

Good point, that makes sense. Renamed.

@JanCaha
Copy link
Contributor Author

JanCaha commented Jul 16, 2025

changed by @wonder-sk requests to drop the decorator and change it to parameter

Copy link
Contributor

@uclaros uclaros left a comment

Choose a reason for hiding this comment

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

Looks good once again!

@uclaros uclaros merged commit 84bac20 into master Jul 16, 2025
4 checks passed
@uclaros uclaros deleted the allow-mc-without-auth-explicitly branch July 16, 2025 17:28
@MarcelGeo MarcelGeo added this to the 0.10.2 milestone Jul 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants