Skip to content
This repository has been archived by the owner on Jun 12, 2021. It is now read-only.

Check if Configuration is valid #70

Open
peppelinux opened this issue Jul 17, 2020 · 6 comments
Open

Check if Configuration is valid #70

peppelinux opened this issue Jul 17, 2020 · 6 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@peppelinux
Copy link
Member

Hello everybody, this is a long-term issue.
During the past year I lived on my skin many jwtconnect breakable upgrades and at this moment an user can still add some options in oidcendpoint configuration.yml, that do not belong anymore to the release he's currently using.

An example is http_params changed then in httpc_params and others as we seen from v0.13.0 to v1.0.1.
I think that a configuration schema validator could help as newcomers users as developers to get some warnings or, in best cases, exception when service starts. This would avoid to run oidcendpoint in a inconsistent way, no more errors during oauth2/oidc sessions would happens.

in django we also use a special command called diffsettings that shows up with configuration fields that would not belong to one's well known in the system core.

A configuration object with a validation method, that starts first of all.
This code would also put a base to the documentation that could be estracted directly from the configuration schema definition.
What else?

@rohe
Copy link
Contributor

rohe commented Jul 18, 2020

That's a very good idea.
I guess it's on my shoulders to implement something like this though I would not mind if someone else stepped up to the plate.

@rohe
Copy link
Contributor

rohe commented Jul 18, 2020

On this issue, a while ago we added develop as the default branch to CryptoJWT and now requests a review before allowing a PR to go through to master.
We should probably do the same for the rest of the stack.

@peppelinux
Copy link
Member Author

On this issue, a while ago we added develop as the default branch to CryptoJWT and now requests a review before allowing a PR to go through to master.
We should probably do the same for the rest of the stack.

Ok, I'll follow this line.
This issue Is related to oidcendpoint only

@peppelinux peppelinux added enhancement New feature or request help wanted Extra attention is needed labels Jul 18, 2020
@rohe
Copy link
Contributor

rohe commented Jul 19, 2020

Sure, but I start to see it as a system (me :-)) error.

@peppelinux
Copy link
Member Author

Could it be out of scope here?
oidc-op would be the the point to deal from, instead of oidcendpoint.
https://github.com/IdentityPython/oidc-op/blob/master/src/oidcop/configure.py

I'd suggest two method in that Class: .clean and .validate. The first run .validate and return a configuration without the unknow/invalid definition (a clean configuration) or {} is something went wrong. The latter would do:

  • for each element (key) - in the root and its childs - matches the model class (defined in oidcendpoint as configuration_models.py?), init it and .validate (in this latter class). The configuration node modelclass would inherit a class AbstractConfigurationNode and this would have these methods: __init__, validate and setup (NotImplemented). This latter would, for example, do some tasks like jwks creation (if readonly is false) or other kind of things, it would be executed only if validate returns True.

A ConfigurationNodeClass could be for example this:

class OpConfNode(AbstractConfigurationNode):
    server_info = dict(type=dict, 
                                 description='Documentation here',
                                 required_fields=['issuer', 'session_key','grant_types_supported'],
                                 optional_fields=[...],
                                 class=OpServerInfoConfNode)
    
     __init__(self, conf: Dict) -> None:
       # conf would be the only `op` value in the general conf
       # if not isinstance(dict) -> {} ... decide if do a decorator for this
       self.description # here the documentation       
       
   validate(self):
       for i in self.conf: 
           # (matches and initialize some more ConfNode) and validate them
           if not hasattr(self, i) -> not defined -> raise Exception 
           ...

   setup(self):
        do things.
        example: for each client, get their jwks and build the keyjar ... many other 

Could it be a starting point or would we like to spend some more words on this?

@peppelinux
Copy link
Member Author

in idpy we discussed about it
https://pydantic-docs.helpmanual.io/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants