CORS for a Twitch Extension
Tags: devjournal, security
Published: 2021-11-20 05:33:36

This article is the fourth in a multi-part series to walk through the creation of a Twitch extension. For the fourth part, the goal is to refactor the CORS headers.

To go directly to the project, the source code repository is available at
{% github shrmpy/pavlok no-readme %}
and
{% github shrmpy/pavlok-panel no-readme %}

__Requirements:__
- Twitch extension client ID

## &sect; Overview
{% codepen https://codepen.io/shrmpy/pen/mdMobOZ %}

## &sect; Headers
### `preprocess` flow
{% codepen https://codepen.io/shrmpy/pen/qBXjmJP %}

### `enableCors` flow
{% codepen https://codepen.io/shrmpy/pen/PoKjOBq %}

- The wildcard (`*`) in the `Access-Control-Allow-Origin` header is the primary change in this refactor work. It is time to restrict the origin to the hosting server (`ID.ext-twitch.tv`) of the Twitch extension.
- Another change that should not add extra scope, is to remove `DELETE` from the `Access-Control-Allow-Methods` header.

## &sect; Test
Start the refactor by adding the [new test](https://gist.github.com/shrmpy/1961ee953b5f9b84d762919dc346998f):
```go
func TestAccessControlAllowOrigin(t *testing.T) {
        // prepare data
        conf := ebs.NewConfig()
        conf.ExtensionId("HOSTNAME-TEST")
        expectMethods := "POST, GET, OPTIONS, PUT"
        expectHeaders := "Accept, Content-Type, Content-Length, Accept-Encoding, X-CSRF-Token, Authorization"
        expectOrigin := "https://HOSTNAME-TEST.ext-twitch.tv"
        req := newTestRequest("GET")

        // run handler logic
        result, err := ebs.MiddlewareCORS(conf, handler)(req)
        assert.IsType(t, nil, err)
        assert.Equal(t, http.StatusOK, result.StatusCode)

        // check for expected CORS
        assert.Equal(t, expectMethods, result.Headers["Access-Control-Allow-Methods"])
        assert.Equal(t, expectHeaders, result.Headers["Access-Control-Allow-Headers"])
        assert.Equal(t, expectOrigin, result.Headers["Access-Control-Allow-Origin"])
}
```

Calling the test runner will lead to compile errors:
```bash
go test $PWD/cmd/auth
```

## &sect; Refactor
Add the new package files to define the [configuration](https://gist.github.com/shrmpy/68d1d82b21016843267e7acfdb748f5e) and [middleware](https://gist.github.com/shrmpy/536a78d1b3e79b7bac50a1af2deb8672):
```go
package ebs

import (
	"fmt"
	"os"
)

// configuration
// to make environment variables available to testing
const EXTENSION_ID = "EXTENSION_ID"

type Config struct {
	extensionId string
}

func NewConfig() *Config {
	return &Config{}
}

func (c *Config) ExtensionId(id string) {
	c.extensionId = id
}

func (c *Config) Hostname() string {
	// format the hostname for the CORS allow-origin
	// 1. For Netlify, EXTENSION_ID environment variable should be defined
	// 2. Locally for testing, rely on configuration field
	if c.extensionId != "" {
		return fmt.Sprintf("https://%s.ext-twitch.tv", c.extensionId)
	}

	cid := os.Getenv(EXTENSION_ID)
	if cid != "" {
		c.extensionId = cid
		return fmt.Sprintf("https://%s.ext-twitch.tv", c.extensionId)
	}

	return ""
}
```
```go
package ebs

import (
	"net/http"

	"github.com/aws/aws-lambda-go/events"
)

type HandlerFunc func(events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error)

/*
func MiddlewareTemplate(next HandlerFunc) HandlerFunc {
	return func(ev events.APIGatewayProxyRequest)
			(events.APIGatewayProxyResponse, error) {
		return next(ev)
	}
}
*/

func MiddlewareCORS(conf *Config, next HandlerFunc) HandlerFunc {
	return func(ev events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) {
		// preflight check is short-circuited
		if ev.HTTPMethod == "OPTIONS" {
			return blankResponse(conf, "", http.StatusOK), nil
		}
		// without next, just act same as preflight
		if next == nil {
			return blankResponse(conf, "", http.StatusOK), nil
		}

		// run next handler along chain
		resp, err := next(ev)
		if err != nil {
			return resp, err
		}

		// post-process
		resp.Headers = enableCors(conf, resp.Headers)

		return resp, nil
	}
}

func enableCors(conf *Config, headers map[string]string) map[string]string {
	m := map[string]string{
		"Access-Control-Allow-Origin":  conf.Hostname(),
		"Access-Control-Allow-Methods": "POST, GET, OPTIONS, PUT",
		"Access-Control-Allow-Headers": "Accept, Content-Type, Content-Length, Accept-Encoding, X-CSRF-Token, Authorization",
	}
	// TODO merge, if CORS headers exist
	for key, val := range headers {
		m[key] = val
	}
	return m
}

func blankResponse(conf *Config, descr string, status int) events.APIGatewayProxyResponse {

	h := enableCors(conf, make(map[string]string))
	h["Content-Type"] = "application/json"

	return events.APIGatewayProxyResponse{

		Headers:    h,
		StatusCode: status,
	}
}
```


There will be references to `enableCors` in `preprocess.go` and `main.go` files that need to be cleaned-up. 

The changes also break one of the existing tests. So fix the old test for the [preflight request](https://gist.github.com/shrmpy/33635208047e79daf1adab393aff3689)
```go
func TestPreflight(t *testing.T) {
        // prep test data
        conf := ebs.NewConfig()
        expectMethods := "POST, GET, OPTIONS, PUT"
        expectHeaders := "Accept, Content-Type, Content-Length, Accept-Encoding, X-CSRF-Token, Authorization"
        expectOrigin := ""
        req := newTestRequest("OPTIONS")

        // run the handler logic
        result, err := ebs.MiddlewareCORS(conf, handler)(req)
        assert.IsType(t, nil, err)
        assert.Equal(t, http.StatusOK, result.StatusCode)
        // check for expected CORS
        assert.Equal(t, expectMethods, result.Headers["Access-Control-Allow-Methods"])
        assert.Equal(t, expectHeaders, result.Headers["Access-Control-Allow-Headers"])
        assert.Equal(t, expectOrigin, result.Headers["Access-Control-Allow-Origin"])
}
```

Afterwards, the compile should be successful. Plus calling the test runner this time should have zero fails. All done? not yet. *Even though the tests pass,* the middleware has not been applied to the original handler. Go to the [main.go file](https://gist.github.com/shrmpy/ab950a1ee78235c2bf28146434f208e9) and adjust the `init()` and `lambda.Start()`:
```go
var conf *ebs.Config

func init() {
        conf = ebs.NewConfig()
        secret := os.Getenv("EXTENSION_SECRET")
        helper = newService(decodeSecret(secret))
}

func main() {
        lambda.Start(
                ebs.MiddlewareCORS(conf,
                        handler,
                ),
        )
}
```

Finally, the new configuration also expects a new environment variable `EXTENSION_ID`. Go to the Netlify __Site settings | Build & deploy | Environment__ page. Click the __Add__ variable button. Name it `EXTENSION_ID` and paste the Twitch extension client ID.

Remember coding standards before saving the changes:
```bash
go fmt $PWD
go fmt $PWD/cmd/auth
git add config.go middleware.go $PWD/cmd/auth
git commit -m'refactor CORS allow-origin'
git push origin gh-issue-NNN
```

____
## &#42; Notes, Lessons, Monologue
*Why change the `Access-Control-Allow-Origin`?* We used the wildcard (`*`) in the early iterations, in order to make the requests work. At the time, there were CORS errors to overcome and without knowing the correct values required, we chose to *allow all*. Now it's time to restrict access for security. So we learned that the Twitch extension is hosted from the `ID.ext-twitch.tv` server and this would be the correct value for the Allow-Origin header.

*Why the middleware?* It was in our backlog. So it was a matter of when. For this refactor, the idea was to intercept the response to shape the headers (that affect CORS). To intercept the response, do processing, and then continue the response flow, fits the description of middleware. The other benefit of middleware is consolidation and uncluttering the business logic. Before, we checked for preflight in `preprocess`, repeated basic responses, and made a direct call to `enableCors` from the main handler. Now CORS header logic is in one place, `ebs.MiddlewareCORS`.

*Why did the test pass before the `init` and `lambda.Start` was patched?* is the test pointless? The test only covers the middleware for the inputs supplied. The test doesn't execute the `main()` or `init()` functions. It may seem pointless, which is important to pause and reflect. Writing the test forced the design for a way to control the value of `EXTENSION_ID`. Before now, an environment variable was the first choice. So thinking test first, we knew we needed another approach because assigning the environment variable in test scaffolding is not self-contained; the test would need to push any existing environment onto some stack before test run, then pop the environment after tests finish. The environment requires this kind of management because we don't want the test to clobber the variable of the host's environment. Even this precaution isn't self-contained because what if you run tests in parallel? Each test will step on each others' environment variable assignment. A very wordy way to say that's why we created the configuration in this refactor. It might appear as if the configuration struct is an one-off just for the test, but the real value is that it forced us to undertake the decoupling.

*Why not use dot env files?* Honestly, I didn't think of it. At the time, I considered TOML/YAML for the configuration, and decided it was overkill. Remember that we want to do the *minimum* to make a test green. The [config.go](https://gist.github.com/shrmpy/68d1d82b21016843267e7acfdb748f5e) that we defined is lean in the current incarnation. Down the road, it may be the case that [dot env](https://github.com/joho/godotenv) files will be the solution that scales.


*What does the call `ebs.MiddlewareCORS(conf, handler)(req)` do?* why is the `lambda.Start` different? In the test, this line invokes the function wrapped by the middleware. The invoke uses the `req` variable as the parameter to that function. With the `lambda.Start`, the *function pointer* is being supplied. That reference can be resolved at a later time.

*Why pass the configuration as a parameter to the middleware call?* This is the "trick". We needed a way to specify a setting in the handler. Before writing the test, this wasn't an issue since using an environment variable has global scope; the handler would have access to the variable. Inside the test, we need to specify the setting and supply it to the handler without using globals. So the configuration becomes the parameter.