buffoonery.nimble

@@ -1,6 +1,6 @@
 # Package
 
-version       = "0.4.7"
+version       = "0.4.1"
 author        = "Jonathan Bernard"
 description   = "Jonathan's opinionated extensions and auth layer for Jester."
 license       = "MIT"
@@ -15,7 +15,7 @@ requires "nim >= 1.6.2"
 requires @["bcrypt", "mummy", "uuids", "webby"]
 
 # from https://git.jdb-software.com/jdb/nim-packages
-requires @["jwt_full >= 0.2.0"]
+requires @["jwt_full >= 0.2.0", "namespaced_logging >= 0.3.0"]
 
 task unittest, "Runs the unit test suite.":
   exec "nim c -r test/runner"

src/buffoonery.nim

@@ -1,2 +1,5 @@
-import buffoonery/[apierror, apiutils, auth, jsonutils]
+import buffoonery/apierror,
+       buffoonery/apiutils,
+       buffoonery/auth,
+       buffoonery/jsonutils
 export apierror, apiutils, auth, jsonutils

src/buffoonery/apierror.nim

@@ -11,9 +11,6 @@ proc newApiError*(parent: ref Exception = nil, respCode: HttpCode, respMsg: stri
   result.respCode = respCode
   result.respMsg = respMsg
 
-  if not parent.isNil:
-    result.trace &= parent.trace
-
 
 proc raiseApiError*(respCode: HttpCode, respMsg: string, msg = "") =
   var apiError = newApiError(

src/buffoonery/apiutils.nim

@@ -1,5 +1,5 @@
-import std/[json, jsonutils, options, sequtils, strtabs, strutils]
-import mummy, webby
+import std/[json, jsonutils, logging, options, sequtils, strtabs, strutils]
+import mummy, namespaced_logging, webby
 
 import std/httpcore except HttpHeaders
 
@@ -7,6 +7,12 @@ import ./apierror
 
 const CONTENT_TYPE_JSON* = "application/json"
 
+var logNs {.threadvar.}: LoggingNamespace
+
+template log(): untyped =
+  if logNs.isNil: logNs = getLoggerForNamespace("buffoonery/apiutils", lvlDebug)
+  logNs
+
 ## Response Utilities
 ## ------------------
 
@@ -59,9 +65,11 @@ proc makeCorsHeaders*(
       }
     else:
       if reqOrigin.isSome:
-        @{"X-Invalid-Origin-Details": "Unrecognized origin '" & reqOrigin.get & "'."}
+        log().debug "Unrecognized Origin '" & reqOrigin.get & "', excluding CORS headers."
       else:
-        @{"X-Invalid-Origin-Details": "Missing Origin."}
+        log().debug "No Origin supplied, excluding CORS headers."
+      log().debug "Valid origins: " & allowedOrigins.join(", ")
+      @{:}
 
 
 proc makeCorsHeaders*(

src/buffoonery/auth.nim

@@ -1,17 +1,17 @@
-import std/[cookies, json, options, sequtils, strtabs, strutils, tables, times]
-import mummy, uuids, webby
+import std/[cookies, json, logging, options, sequtils, strtabs,
+            strutils, tables, times]
+import mummy, namespaced_logging, uuids, webby
 import std/httpclient except HttpHeaders
 
 import jwt_full, jwt_full/encoding
 
-import ./[apiutils,jsonutils]
+import ./apiutils, ./jsonutils
 
 const SUPPORTED_SIGNATURE_ALGORITHMS = @[ HS256, RS256 ]
 
 
 type
   AuthError* = object of CatchableError
-    additionalInfo*: Option[TableRef[string, JsonNode]]
 
   ApiAuthContext* = ref object
     appDomain*: string            ## Application domain for session cookies
@@ -28,18 +28,11 @@ type
 
     issuerKeys: TableRef[string, JwkSet]
 
+var logNs {.threadvar.}: LoggingNamespace
 
-proc failAuth*[T](
-    reason: string,
-    additionalInfo: TableRef[string, T],
-    parentException: ref Exception = nil) =
-  ## Syntactic sugar to raise an AuthError. Reason will be the exception
-  ## message and should be considered an internal message.
-  let err = newException(AuthError, reason, parentException)
-  err.additionalInfo = some(newTable[string, JsonNode]())
-  for key, val in additionalInfo.pairs:
-    err.additionalInfo.get[key] = %val
-  raise err
+template log(): untyped =
+  if logNs.isNil: logNs = getLoggerForNamespace("buffoonery/auth", lvlDebug)
+  logNs
 
 
 proc failAuth*(reason: string, parentException: ref Exception = nil) =
@@ -77,28 +70,24 @@ proc initApiAuthContext*(
 proc fetchJWKs(openIdConfigUrl: string): JwkSet {.gcsafe.} =
   ## Fetch signing keys for an OAuth issuer. `openIdConfigUrl` is expected to
   ## be a well-known URL (ISSUER_BASE/.well-known/openid-configuration)
-  var jwksKeysURI: string
   try:
     let http = newHttpClient()
 
     # Inspect the OAuth metadata via the well-known address.
+    log().debug "fetchJwks: Fetching metadata from " & openIdConfigUrl
     let metadata = parseJson(http.getContent(openIdConfigUrl))
 
     # Fetch the keys from the jwk_keys URI.
-    jwksKeysURI = metadata.getOrFail("jwks_uri").getStr
+    let jwksKeysURI = metadata.getOrFail("jwks_uri").getStr
+    debug "fetchJwks: Fetching JWKs from " & jwksKeysURI
     let jwksKeys = parseJson(http.getContent(jwksKeysURI))
 
     # Parse and load the keys provided.
     return initJwkSet(jwksKeys)
 
-  except Exception:
-    #failAuth "unable to fetch isser signing keys"
-    failAuth(
-      reason = "unable to fetch isser signing keys",
-      additionalInfo = newTable[string, string]({
-        "openIdConfigUrl": openIdConfigUrl,
-        "jwksKeysURI": jwksKeysURI }),
-      parentException = getCurrentException())
+  except:
+    log().error "unable to fetch issuer signing keys: " & getCurrentExceptionMsg()
+    failAuth "unable to fetch isser signing keys"
 
 
 proc addSigningKeys*(ctx: ApiAuthContext, issuer: string, keySet: JwkSet): void =
@@ -108,6 +97,7 @@ proc addSigningKeys*(ctx: ApiAuthContext, issuer: string, keySet: JwkSet): void
     if ctx.issuerKeys.isNil: ctx.issuerKeys = newTable[string, JwkSet]()
     ctx.issuerKeys[issuer] = keySet
   except:
+    log().error "unable to add a set of signing keys: " & getCurrentExceptionMsg()
     raise getCurrentException()
 
 
@@ -121,8 +111,8 @@ proc findSigningKey*(ctx: ApiAuthContext, jwt: JWT, allowFetch = true): JWK {.gc
   ## [OpenID Connect standard discovery mechanism](https://openid.net/specs/openid-connect-discovery-1_0.html)
 
   try:
-    if jwt.claims.iss.isNone: failAuth "JWT is missing 'iss' claim."
-    if jwt.header.kid.isNone: failAuth "JWT is missing 'kid' header."
+    if jwt.claims.iss.isNone: failAuth "Missing 'iss' claim."
+    if jwt.header.kid.isNone: failAuth "Missing 'kid' header."
 
     if ctx.issuerKeys.isNil: ctx.issuerKeys = newTable[string, JwkSet]()
 
@@ -141,13 +131,10 @@ proc findSigningKey*(ctx: ApiAuthContext, jwt: JWT, allowFetch = true): JWK {.gc
         fetchJWKs(jwtIssuer & "/.well-known/openid-configuration")
       return ctx.findSigningKey(jwt, false)
 
-    failAuth(
-      reason = "unable to find JWT signing key",
-      additionalInfo = newTable[string, string]({
-        "jwtIssuer": jwtIssuer,
-        "jwtKid": jwt.header.kid.get}))
+    failAuth "unable to find JWT signing key"
 
   except:
+    log().error "unable to find JWT signing key: " & getCurrentExceptionMsg()
     failAuth("unable to find JWT signing key", getCurrentException())
 
 
@@ -155,31 +142,25 @@ proc validateJWT*(ctx: ApiAuthContext, jwt: JWT) =
   ## Given a JWT, validate that it is a well-formed JWT, validate the issuer's
   ## signature on the token, and validate all the claims that it preesnts.
   try:
-    if jwt.claims.iss.isNone: failAuth "JWT is missing 'iss' claim."
+    log().debug "Validating JWT: " & $jwt
+    if jwt.claims.iss.isNone: failAuth "Missing 'iss' claim."
     let jwtIssuer = jwt.claims.iss.get
 
     if not ctx.trustedIssuers.contains(jwtIssuer):
-      failAuth(
-        reason = "We don't trust the JWT's issuer.",
-        additionalInfo = newTable[string, JsonNode]({
-          "issuer": %jwtIssuer,
-          "trustedIssuers": %ctx.trustedIssuers}))
+      failAuth "JWT is issued by $# but we only trust $#" %
+        [jwtIssuer, $ctx.trustedIssuers]
 
-    if jwt.header.alg.isNone: failAuth "JWT is missing 'alg' header property."
-    if jwt.claims.aud.isNone: failAuth "JWT is missing 'aud' claim."
-    if jwt.claims.iss.isNone: failAuth "JWT is missing 'iss' claim."
-    if jwt.claims.sub.isNone: failAuth "JWT is missing 'sub' claim."
-    if jwt.claims.exp.isNone: failAuth "JWT is missing or invalid 'exp' claim."
+    if jwt.header.alg.isNone: failAuth "Missing 'alg' header property."
+    if jwt.claims.aud.isNone: failAuth "Missing 'aud' claim."
+    if jwt.claims.iss.isNone: failAuth "Missing 'iss' claim."
+    if jwt.claims.sub.isNone: failAuth "Missing 'sub' claim."
+    if jwt.claims.exp.isNone: failAuth "Missing or invalid 'exp' claim."
 
-    if jwt.claims["aud"].get.kind == JString:
-      # If the token is for a single audience, check that it is for us.
-      if not ctx.validAudiences.contains(jwt.claims.aud.get):
-        failAuth "JWT is not for us (invalid audience)."
-    elif jwt.claims["aud"].get.kind == JArray:
-      # If the token is for multiple audiences, check that at least one is for us.
-      let auds = jwt.claims["aud"].get.getElems
-      if not auds.anyIt(ctx.validAudiences.contains(it.getStr)):
-        failAuth "JWT is not for us (invalid audience)."
+    if not ctx.validAudiences.contains(jwt.claims.aud.get):
+      log().debug(
+        "Valid audiences: $#\ttoken audience: $#" %
+        [$ctx.validAudiences, jwt.claims.aud.get])
+      failAuth "JWT is not for us (invalid audience)."
 
     let signingAlgorithm = jwt.header.alg.get
 
@@ -195,7 +176,7 @@ proc validateJWT*(ctx: ApiAuthContext, jwt: JWT) =
     failAuth(getCurrentExceptionMsg(), getCurrentException())
 
 
-proc extractValidJwt*(ctx: ApiAuthContext, req: Request, validateCsrf = true): JWT =
+proc extractValidJwt*(ctx: ApiAuthContext, req: Request): JWT =
   ## Extracts a valid JWT representing the user's authentication and
   ## authorization details, if present. If there are no valid credentials an
   ## exception is raised.
@@ -219,12 +200,6 @@ proc extractValidJwt*(ctx: ApiAuthContext, req: Request, validateCsrf = true): J
   ##
   ##   In the split-cookie mode we also check that the `csrfToken` claim in the
   ##   JWT payload matches the CSRF value passed via the `X-CSRF-TOKEN` header.
-  ##   This CSRF check can be disabled by setting `validateCsrf` to `false`.
-  ##   This option is proivded to support occasional use-cases where you want to
-  ##   be able to serve a request using cookie auth when the client can't set
-  ##   custom headers (e.g. a simple link from an <a> tag). Obviously, this is a
-  ##   security risk and should only be used with caution with a full
-  ##   understanding of the risk.
 
   try:
     if req.headers.contains("Authorization"):
@@ -248,17 +223,12 @@ proc extractValidJwt*(ctx: ApiAuthContext, req: Request, validateCsrf = true): J
 
       # Because this is a web session, check that the CSRF is present and
       # matches.
-      if validateCsrf:
-        if not req.headers.contains("X-CSRF-TOKEN") or
-           not result.claims["csrfToken"].isSome:
-          failAuth "missing CSRF token"
+      if not req.headers.contains("X-CSRF-TOKEN") or
+         not result.claims["csrfToken"].isSome:
+        failAuth "missing CSRF token"
 
-        if req.headers["X-CSRF-TOKEN"] != result.claims["csrfToken"].get.getStr(""):
-          failAuth(
-            reason = "invalid CSRF token",
-            additionalInfo = newTable[string, string]({
-              "header": req.headers["X-CSRF-TOKEN"],
-              "jwt": result.claims["csrfToken"].get.getStr("")}))
+      if req.headers["X-CSRF-TOKEN"] != result.claims["csrfToken"].get.getStr(""):
+        failAuth "invalid CSRF token"
 
     else: failAuth "no auth token, no Authorization or Cookie headers"