Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: respect explicit allowedOrigins configurations #255

Closed

Conversation

dbjorge
Copy link
Contributor

@dbjorge dbjorge commented May 18, 2021

#240 included a change to have the assorted integration libraries configure a default allowedOrigins policy, but in doing so, it overrides any explicitly-configured allowedOrigins settings configured by a library consumer. We have an @axe-core/puppeteer consumer (https://github.com/microsoft/accessibility-insights-service) where we'd like to be able to override the default policy; this PR implements that support by moving the configure call which sets the default to before the configure call with user-provided configuration.

Our team technically only needs this for @axe-core/puppeteer, but for consistency's sake I've included similar changes in each of the integration libraries which already included support for arbitrary configure calls (puppeteer, react, webdriverjs, but not webdriverio, since it doesn't appear to support an arbitrary configure spec).

We'd really appreciate if this could get in for the 4.2.1 release (@michael-siek , it looks like you're running this?) - we'd like to be able to use allowedOrigins as part of picking up v4.2.x if possible.

@@ -27,7 +27,7 @@ describe('doc-dylang.html', function () {
before(async function () {
// const app: express.Application = express()
const app: express.Application = express();
app.use(express.static(path.resolve(__dirname, '..', 'fixtures')));
app.use(express.static(path.resolve(__dirname, '..', 'fixtures')));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatting changes to the existing tests were recommended by npm run fmt when I ran it as part of validating the new test

Copy link

@lisli1 lisli1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@michael-siek michael-siek requested a review from a team May 18, 2021 23:24
Copy link
Member

@stephenmathieson stephenmathieson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 from me. I thought it was odd that we weren't allowing this to be configurable elsewhere. Will need an OK from Micheal tho.

@stephenmathieson stephenmathieson requested review from michael-siek and a team May 19, 2021 00:35
@WilcoFiers
Copy link
Contributor

We are working on features for axe-core 4.3 which will make allowedOrigins irrelevant for Selenium / Puppeteer. We did not add allowedOrigins as a configurable option because we would have to remove it again once axe-core 4.3 came out. That would be a breaking change that would require us to go to 5.0, which we can not do for a variety of reasons.

For axe-core 4.2 there are a few options available to let you configure what frames run. You can exclude specific frames, or all frames using the exclude option. You can also use the iframes option, set it to false, which turns off all frame testing. And to prevent less information getting passed between frames there is the noHtml option.

@dbjorge
Copy link
Contributor Author

dbjorge commented May 19, 2021

Thanks for sharing that, Wilco! We'll pursue a workaround until the 4.3 changes are available.

@dbjorge dbjorge closed this May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants