Use of authentication methods other than Basic Auth

[cimnine]

The service broker API spec defines that «it is RECOMMENDED that all communications between a Platform and a Service Broker are […] authenticated.» Also that «Platforms and Service Brokers MAY agree on an authentication mechanism other than basic authentication, but the specific agreements are not covered by this specification.»

This stands in contrast to the implementation of this project, which mandates Basic Auth, at least when initialized using api.New(…).

In our project, we would like to implement authentication based on Bearer Tokens. I'm tasked to implement this.

Currently, my only option is to replicate the api.New(…) in our own code. But I would like to avoid that, because it increases the possibility that something breaks unexpectedly when we update to a later version of the brokerapi:
We would have to establish some means to check that our replication of api.New(…) still matches the upstream version, except for the authentication part.

I therefore propose a refactoring of that method. With backwards-compatibility in mind, I suggest to introduce a new function func NewWithCustomAuth(serviceBroker ServiceBroker, logger lager.Logger, authMiddleware http.Handler) in api.go like so:

func New(serviceBroker ServiceBroker, logger lager.Logger, brokerCredentials BrokerCredentials) http.Handler {
	authMiddleware := auth.NewWrapper(brokerCredentials.Username, brokerCredentials.Password).Wrap
	return NewWithCustomAuth(serviceBroker, logger, authMiddleware)
}

func NewWithCustomAuth(serviceBroker ServiceBroker, logger lager.Logger, authMiddleware mux.MiddlewareFunc) http.Handler {
	router := mux.NewRouter()

	AttachRoutes(router, serviceBroker, logger)

	apiVersionMiddleware := middlewares.APIVersionMiddleware{LoggerFactory: logger}

	router.Use(middlewares.AddCorrelationIDToContext)
	router.Use(authMiddleware)
	router.Use(middlewares.AddOriginatingIdentityToContext)
	router.Use(apiVersionMiddleware.ValidateAPIVersionHdr)
	router.Use(middlewares.AddInfoLocationToContext)
	router.Use(middlewares.AddRequestIdentityToContext)

	return router
}

Perhaps you find a better name for NewWithCustomAuth, as it's not pretty. Yet it does convey the message what it differentiates from func New(…).

Now:

1. Did I miss a way to implement authentication using Bearer Tokens without modifying or replicating api.New(…)?
2. Would you welcome a PR that changes the implementation to the proposed implementation?
3. What else did I miss?

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/177739656

The labels on this github issue will be updated when the story is started.

[FelisiaM]

Hi @cimnine

Thanks for opening the issue.
Your proposed solution sounds reasonable. We would be open to reviewing your PR if you submit one.

Thanks,
Felisia

[FelisiaM]

Hi @cimnine

Are you still able to provide a PR? We are considering releasing a new version in the next few days and we could include your change there.

Thanks,
Felisia

[cimnine]

If I can't submit a PR by tomorrow, it will probably take a while. In this case, please proceed with a release regardless.

[cimnine]

See #168 for a PR.

[FelisiaM]

Thank you for your contribution @cimnine!