Is the oauth state parameter mishandled?

[egurnick]

The oauth "state" parameter, used to prevent csrf attacks, is generated in fab like so for each auth request

Flask-AppBuilder/flask_appbuilder/security/views.py

Lines 622 to 626 in e7692d1

	state = jwt.encode(
	request.args.to_dict(flat=False),
	self.appbuilder.app.config["SECRET_KEY"],
	algorithm="HS256",
	)

Where config["SECRET_KEY"] is defined in config.py (and should be set by the developer)

Flask-AppBuilder/bin/config.py

Line 5 in 27b15e5

SECRET_KEY = '\2\1thisismyscretkey\1\2\e\y\y\h'

and request.args is defined as
https://github.com/pallets/werkzeug/blob/main/src/werkzeug/sansio/request.py#L170-L185

    def args(self) -> "MultiDict[str, str]":
        """The parsed URL parameters (the part in the URL after the question
        mark).
        By default an
        :class:`~werkzeug.datastructures.ImmutableMultiDict`
        is returned from this function.  This can be changed by setting
        :attr:`parameter_storage_class` to a different type.  This might
        be necessary if the order of the form data is important.
        """
        return url_decode(
            self.query_string,
            self.url_charset,
            errors=self.encoding_errors,
            cls=self.parameter_storage_class,
        )

Thus, the state parameter is essentially a constant and has the same value over multiple auth requests. If config["SECRET_KEY"]` is known, the state parameter is predictable.

The oauth standard states
https://datatracker.ietf.org/doc/html/rfc6749#section-4.2.1

state
RECOMMENDED. An opaque value used by the client to maintain
state between the request and callback. The authorization
server includes this value when redirecting the user-agent back
to the client. The parameter SHOULD be used for preventing
cross-site request forgery as described in Section 10.12.

https://datatracker.ietf.org/doc/html/rfc6749#section-10.12

state (e.g., a hash of the session
cookie used to authenticate the user-agent)

All other implementations I have seen use a randomly generated string for each new auth request

Is this correct usage of the state parameter?

[dpgaspar]

Thank you for reporting.
although it's an opaque value and prevent CSRF, I agree we should be using a random string. I'll make a fix ASAP.

[dpgaspar]

Got a PR up #1932 , @egurnick a review would be great

[dpgaspar]

merged will be included on the next release

[egurnick]

Looks good, nice solution, thank you @dpgaspar !