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.

Sign up for GitHub

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

Jump to bottom

Qmatic plugin does not use mtls #3328

Closed
joeribekker opened this issue Aug 1, 2023 · 6 comments · Fixed by #3488
Closed

Qmatic plugin does not use mtls #3328

joeribekker opened this issue Aug 1, 2023 · 6 comments · Fixed by #3488
Assignees
@sergei-maertens
Labels
bug Something isn't working
Milestone
Release 2.4.0

Comments

@joeribekker
Copy link
Contributor

Product versie / Product version

2.3.0-alpha.0

Omschrijf het probleem / Describe the bug

Certificate can be provided but is not sent along to the server.

Qmatic itself does not use this but when an organization uses a gateway, it can require a certificate.

Stappen om te reproduceren / Steps to reproduce

No response

Verwacht gedrag / Expected behavior

No response

Screen resolution

None

Device

None

OS

None

Browser

No response
@joeribekker joeribekker added bug Something isn't working triage Issue needs to be validated. Remove this label if the issue considered valid. and removed triage Issue needs to be validated. Remove this label if the issue considered valid. labels Aug 1, 2023
joeribekker added a commit that referenced this issue Aug 1, 2023
@joeribekker
🔒 [#3328] Use mtls when certs are configured for Qmatic
1781cec
@joeribekker joeribekker mentioned this issue Aug 1, 2023
Closed
@joeribekker
Copy link
Contributor Author

Refinement:

  1. Phase 1: add generic utility to extract TLS/auth parameters from a service-like object (protocol?) to requests calls
  2. Phase 2: provide own requests session subclass/wrapper taking some sort of context to automatically populate these kwargs (to be designed)

@sergei-maertens
Copy link
Member

See also #3299 (comment)

So I'm marking this as a wontfix since we're looking to remove Qmatic from the codebase.

@sergei-maertens sergei-maertens closed this as not planned Won't fix, can't repro, duplicate, stale Aug 21, 2023
@joeribekker
Copy link
Contributor Author

It is used but no immediate need for 2.3.0

@joeribekker joeribekker reopened this Aug 21, 2023
@sergei-maertens
Copy link
Member

It is used but no immediate need for 2.3.0

Can we then please discuss when you're back what the plan is? I had the impression only LV used this and they no longer use it through OF?

@LaurensBurger
Copy link
Collaborator

DB uses it.

@sergei-maertens sergei-maertens self-assigned this Sep 6, 2023
@sergei-maertens
Copy link
Member

All the places that support client certificate configuration for mTLS:

  • soap.SoapService
    • jcc.JCCConfig
  • stuf.StufService
    • stuf_bg.StufBGConfig
    • stuf_zds.StufZDSConfig
  • zgw_consumers.Service
    • qmatic.QmaticConfig
    • bag.BAGConfig
    • brp.BRPConfig
    • kadaster.KadasterApiConfig
    • kvk.KVKConfig
    • haalcentraal.HaalCentraalConfig
    • objects_api.ObjectsAPIConfig
    • zgw_apis.ZGWApiGroupConfig
    • variables.ServiceFetchConfiguration

Anything that uses this config to create a client must account for potential mTLS and/or other auth params

sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
⚗️ [#3328] Explore initial architecture/API
1cdab4b
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
✅ [#3328] Add tests for desired behaviour
a969303
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
✨ [#3328] Support configuration-time session attrs at request time
2090167
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
✅ [#3328] Add some property-based tests to cover requests' API
dd08f8b
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
✨ [#3328] Support relative URLs and validate absolute URLs
0f8e4a6
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
✨ [#3328] Implement conditional session closing
84b6ea6 
Taken from the StUF client base implementation, ensure that one-off
requests still close the session.
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
📝 [#3328] Provide some basic documentation in README
8eccc64
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
🧪 [#3328] Add tests for missing mTLS parameters to requests
cddcee7
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
✨ [#3328] Implement APIClient factory for zgw_consumers.Service
dbacaa8
@sergei-maertens sergei-maertens mentioned this issue Sep 18, 2023
Merged
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
✨ [#3328] Implement APIClient factory for zgw_consumers.Service
ca260c1
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
✨ [#3328] Implement APIClient factory for zgw_consumers.Service
91b1fe7
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
✅ [#3328] Add unit tests for zgw_consumers service factory
a331389 
Put the unit tests close to the actual implementation and keep some
smoke tests as regression tests to prove the original issue is indeed
resolved.
sergei-maertens added a commit that referenced this issue Sep 18, 2023
@sergei-maertens
✨ [#3328] Implement zgw_consumers.Service auth types in factory
fcd47bd
sergei-maertens added a commit that referenced this issue Sep 19, 2023
@sergei-maertens
👌 [#3328] PR feedback processing
753575e 
* Cleaned up base URL checking a bit
* Cleaned up hypothesis search strategy
* Clean up context manager code style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sergei-maertens sergei-maertens

Labels
bug Something isn't working
Projects
Development
Archived in project
Milestone
Release 2.4.0
3 participants
@joeribekker @sergei-maertens @LaurensBurger