Support cookie parameters about swift-openapi-generator HOT 17 OPEN

Here is the OpenApi snippet of the route I was referring to (I am setting the cookie in a /login route)

  /refresh-token:
    post:
      operationId: refreshToken
      description: Returns a new access token and optionally changes the station. (for renewal or station change)
      parameters:
        - in: cookie
          name: refreshtoken
          schema: { type: string }
        
      requestBody:
        required: true
        content:
          application/json: 
            schema:
              type: object
              required: 
               - station
              properties:
                station: { type: string }

from swift-openapi-generator.

There is some existing support as part of parameters

At least on the server side I could not find a way to access cookie parameters in the generated request handlers. I can probably work my way around this by using some TaskLocal construct via a middleware, but it sure would be nice to just have typed access in the route handler (or maybe just access to the "raw" request for any not implemented feature yet?)

Use case: A refresh-token endpoint that uses a httpOnly cookie for the actual refresh token (so it's just for that one route).

from swift-openapi-generator.

@sliemeobn thanks for adding your feedback here. It's very useful as we want to prioritise the features based on demand from the community.

This shouldn't be too challenging if someone wants to pick it up, as it will likely be similar to other parameters.

but it sure would be nice to just have typed access in the route handler (or maybe just access to the "raw" request for any not implemented feature yet?)

I don't think we've ever discussed adding an escape hatch because things can usually be worked around in middlewares (as you noted), but maybe we could consider it (would be underscored and subject to removal).

What do you think such an API would look like?

Currently the APIProtocol declares the operations as functions from the Input to Output type and has no knowledge of the HTTP request or response. This protocol is used on the client and server sides. The ServerTransport obviously has access to more information, but it's not clear how we'd get that to the "user function" for the handler implementation, without breaking the symmetry of protocol use on client and server.

from swift-openapi-generator.

To help us all iterate here, @sliemeobn could you paste a short snippet of the cookie parameter definition in YAML here? We can then discuss what the generated code should look like on the client and server.

from swift-openapi-generator.

I am setting the cookie in a /login route

Can you elaborate on that? Are you saying that the call sequence is the following?

1. Call /login route with some other auth method, get back a refresh token that you store locally
2. Call /refresh-token with an optional refreshtoken cookie (what happens if it's not specified?) using the token from the previous call, possibly getting back another cookie value? Is that cookie coming back documented in the OpenAPI doc?

from swift-openapi-generator.

The /login just takes user/password (and station, application specific stuff) as a payload. It returns a short-lived token (for API access) and a Set-Cookie: refershtoken=xxx for renewal (refreshToken with longer expiry).

The application will then renew the access token without requiring a password.
Obviously, it does not have to be done that way, but a scoped httpOnly cookie is quite a good place to store this.

  /login:
    post:
      operationId: login
      requestBody:
        required: true
        content:
          application/json: { schema: { $ref: "#/components/schemas/Login"} }
      responses:
        "200":
          description: Success
          headers:
            Set-Cookie: { schema: { type: string } }
          content:
            application/json: { schema: { $ref: "#/components/schemas/TokenResponse" }}
        "401":
          description: Unauthorized

from swift-openapi-generator.

Gotcha, and the /refresh-token response also has the following?

headers:
    Set-Cookie: { schema: { type: string } }

Just trying to understand the invocation, and how much could be generated and shared between all users of cookies, and how much we should instead put into middlewares 🙂

from swift-openapi-generator.

At the moment the refreshtoken is only issued on /login - when it expires, a password is needed. So /refresh-token will not set a new one.

from swift-openapi-generator.

About the "escape hatch", I am currently playing around with this:

/// Extensions to work around current limitiation on OpenAPI generator
enum RawRequest {
    @TaskLocal static var current: OpenAPIRuntime.Request?
}

/// Make RawRequest.current available as a task-local.
struct RawRequestMiddleware: ServerMiddleware {
    public func intercept(_ request: OpenAPIRuntime.Request, metadata: OpenAPIRuntime.ServerRequestMetadata, operationID _: String, next: (OpenAPIRuntime.Request, OpenAPIRuntime.ServerRequestMetadata) async throws -> OpenAPIRuntime.Response) async throws -> OpenAPIRuntime.Response {
        try await RawRequest.$current.withValue(request) { try await next(request, metadata) }
    }
}

I general, I was thinking about how most web server frameworks provide some form of "storage" on their request structure for attaching all sorts of things and passing around this unwieldy thing down the pipe. But as this was all conceived before structured concurrency and TaskLocals, and I quite like the "clean" functional way of the generated handler being all "you can only do what you define in the spec!".

But without storage on the request, it'll be tough to use middleware to "pre-process" requests in some reusable form without TaskLocals... So, not sure if that ends up being any better. 🤔

from swift-openapi-generator.

Right, what you have is roughly what I had in mind.

One difference is that I'd make the middleware extract the value you're interested in, validate it, etc, and then put it into a task local.

If you think about how you'd then test your handler, it'll be easier to inject a specific value instead of having to create a whole request.

Once cookies are supported more properly, I'd expect them to show up on the Input struct, just like all the other parameters.

from swift-openapi-generator.

Adding needs-design as we should probably provide more value than just propagate the values like with headers.

from swift-openapi-generator.

We got a repeated ask for this, and with percent encoding of all headers in 0.2.0 cookies might be even more broken, so pulling into 1.0.

from swift-openapi-generator.

since I just stumbled over the issue of "setting cookies" via a Set-Cookie header, I want to add that a "cookie parameter" feature should also include a proper way for server code to set cookies.

I was using a plain string header Set-Cookie like this:

responses:
        "200":
          description: Success
          headers:
            Set-Cookie: { schema: { type: string } }
          content:
            application/json:
              { schema: { $ref: "#/components/schemas/TokenResponse" } }

and then setting a set cookie string in the application code (like myCookie=something; Foo=Bar; ...)

to my surprise a change to the generator started auto-encoding all header values with URL encoding and broke my API.
obviously, browsers do not understand that - so it becomes quite a middleware-involved dance to set a cookie through the generated server stubs ATM.

bonus thought: I am not sold that always auto-URL-encoding all header values does more good than harm, but I can respect the "OpenAPI correctness first" position. so if anyone else has a status-quo-challenging opinion on this, it might deserve it's own issue.

from swift-openapi-generator.

Thanks for the additional detail, @sliemeobn.

Yes, the URI encoding of headers was a bug fixed in 0.2.0, we previously weren't doing that.

However, since we don't currently treat cookies differently to other headers, this likely broke cookies for some adopters.

Potential incremental support:

1. Support only raw string cookies, not structured ones yet (just emit a diagnostic and skip if encountered).
2. Add support for structured cookies (might make sense to split into a separate issue).

Also, this is likely related to #37, as security schemes can use cookies for storage.

from swift-openapi-generator.

Note that https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie says:

Encoding: Many implementations perform URL encoding on cookie values. However, this is not required by the RFC specification. The URL encoding does help to satisfy the requirements of the characters allowed for .

I can't tell, however, whether they mean encoding the whole header, or just the <cookie-value> portion.

from swift-openapi-generator.

As a workaround for server implementors running in the same issue as @sliemeobn, here's a server middleware you can add that should strip the percent encoding from any outgoing set-cookie headers:

struct SetCookieUnescapingMiddleware: ServerMiddleware {
    func intercept(
        _ request: Request,
        metadata: ServerRequestMetadata,
        operationID: String,
        next: @Sendable (Request, ServerRequestMetadata) async throws -> Response
    ) async throws -> Response {
        var response = try await next(request, metadata)
        response.headerFields = response.headerFields.map { headerField in
            guard headerField.name.lowercased() == "set-cookie" else {
                return headerField
            }
            var headerField = headerField
            headerField.value = headerField.value.removingPercentEncoding ?? headerField.value
            return headerField
        }
        return response
    }
}

Then don't forget to add it to your handler.registerHandlers(..., middlewares: [SetCookieUnescapingMiddleware()]) call.

from swift-openapi-generator.

in case somebody is interested, I hacked together this workaround for my use case for now:

/// Provides Response.setCookies and attaches all added cookies to the response header
public struct SetCookieMiddleware: ServerMiddleware {
    
    public init() {}

    public func intercept(_ request: OpenAPIRuntime.Request, metadata: OpenAPIRuntime.ServerRequestMetadata, operationID: String, next: @Sendable (OpenAPIRuntime.Request, OpenAPIRuntime.ServerRequestMetadata) async throws -> OpenAPIRuntime.Response) async throws -> OpenAPIRuntime.Response {
        try await Response.$_setCookies.withValue(.init()) {
            var response = try await next(request, metadata)

            for cookie in Response.setCookies.cookies {
                response.headerFields.append(.init(name: "Set-Cookie", value: cookie.description))
            }

            return response
        }
    }
}

public extension Response {
    /// Storage for cookies to be set on the response
    class Cookies: @unchecked Sendable {
        var _cookies: NIOLockedValueBox<[SLCookie]> = .init([])
        public var cookies: [SLCookie] { _cookies.withLockedValue { $0 }}

        public func add(_ cookie: SLCookie) {
            _cookies.withLockedValue { $0.append(cookie) }
        }
    }

    @TaskLocal fileprivate static var _setCookies: Cookies?

    // work-around until cookies are supported though openapi-generator
    // https://github.com/apple/swift-openapi-generator/issues/38
    /// Collects cookies and send them via the
    static var setCookies: Cookies {
        precondition(Self._setCookies != nil, "Response.setCookies called outside request handling task, make sure SetCookieMiddleware is used!")
        return Self._setCookies!
    }
}

usage in API code:

            Response.setCookies.add(cookieForSettingRefreshToken(refreshtoken))

            return try .ok(.init(
                body: .json(.init(
                    token: jwt.sign(accessToken)
                ))
            ))

where SLCookie is just a struct that deals with cookie encoding (pretty much like HBCookie from hummingbird)

from swift-openapi-generator.