Add config for local JWT tokens #2568
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2568 +/- ##
==========================================
+ Coverage 54.20% 54.80% +0.60%
==========================================
Files 558 560 +2
Lines 40634 40786 +152
==========================================
+ Hits 22026 22354 +328
+ Misses 18608 18432 -176
... and 26 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Kudos, SonarCloud Quality Gate passed!
|
There was a problem hiding this comment.
This is shaping up now, but there is a problem with how/when the configuration file is being read.
After a clean installation of NAV, many NAV commands (e.g. navdf) now spit out this error message (but continue working):
Error reading jwtconfig: Configuration error: Invalid 'name': 'name' must not be empty
Which is pretty confusing for anyone who hasn't even considered changing jwt.conf yet.
|
Updated so that local JWT config is optional. By default it is commented out, so to activate it you comment in the lines and fill in the correct values. + some other changes. So now error message for missing name etc will only show up if you actually have those lines active |
stveit
force-pushed
the
jwt-local-token-config
branch
from
ff427ea
to
c856028
Compare
other tests cover these things more directly
aka removes the "general" exception check
stveit
force-pushed
the
jwt-local-token-config
branch
from
c856028
to
35079b7
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
|
did a rebase on master to get tests passing. I have a branch with much cleaner commit log, but im keeping it as it is until its approved |
|
Instead of using the ConfigurationError exception for everyyhing, there should be subclasses for each specific thing that can go wrong |
There was a problem hiding this comment.
Good idea with the ConfigurationError wrapping :)
The one thing I remember holding back from my previous review is this: Generally, all the config that is specific to the web parts of NAV is located in the etc/webfront/ directory. IMHO, the jwt.conf file should probably be located there as well (otherwise, we should start looking at restructuring everything inside etc/webfront/, which is out of scope in this PR)
I would tend to agree. But I suggest doing that in a separate PR, rather than complicate and prolong this one :) Make an issue! |
|
Kudos, SonarCloud Quality Gate passed!
|
Add config for info necessary for jwt tokens issued by the nav instance itself.
public key for configuring the oidc module, private key for signing and name for setting
'iss' and 'aud' claims.
Makes these values available through the JWTConf class.
Wraps errors are ConfigurationError, so other code that will later access the private key etc. only have to deal with that and not stuff like configparser.NoSectionError.