-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Sharing embeds with safe parameters #3495
Merged
Merged
Changes from all commits
Commits
Show all changes
61 commits
Select commit
Hold shift + click to select a range
3535dfd
change has_access and require_access signatures to work with the obje…
fc02fe7
change has_access and require_access signatures to work with the obje…
eef39de
use the textless endpoint (/api/queries/:id/results) for pristine
3c1ed5c
Revert "use the textless endpoint (/api/queries/:id/results) for pris…
a3c543c
go to textless /api/queries/:id/results by default
b99f114
change `run_query`'s signature to accept a ParameterizedQuery instead of
2c48d36
raise HTTP 400 when receiving invalid parameter values. Fixes #3394
d03d9e1
support querystring params
637e985
extract coercing of numbers to function, along with a friendlier
8776b40
wire embeds to textless endpoint
7d9daa7
allow users with view_only permissions to execute queries on the
1eeeef8
enqueue jobs for ApiUsers
475f6f8
add parameters component for embeds
ca9e665
include existing parameters in embed code
82ff905
fetch correct values for json requests
f4c22ca
remove previous embed parameter code
18c1876
rename `id` to `user_id`
7542ece
support executing queries using Query api_keys by instantiating an Ap…
0eb1963
bring back ALLOW_PARAMETERS_IN_EMBEDS (with link on deprecation comin…
18d5b94
show deprecation messages for ALLOW_PARAMETERS_IN_EMBEDS. Also, move
54d7d82
add link to forum message on setting deprecation
34f7762
rephrase deprecation message
96d56ee
add link to forum message regarding embed deprecation
e19517c
change API to /api/queries/:id/dropdowns/:dropdown_id
ef286a7
split to 2 different dropdown endpoints and implement the second
87a9030
add test cases for /api/queries/:id/dropdowns/:id
b279b7a
use new /dropdowns endpoint in frontend
aa59a89
first e2e test for sharing embeds
9cc2de5
Pleasing the CodeClimate overlords
0fce9c3
All glory to CodeClimate
a40bc29
change has_access and require_access signatures to work with the obje…
353bb9f
split has_access between normal users and ApiKey users
e95546b
remove residues from bad rebase
735a829
allow access to safe queries via api keys
55eb7ce
Merge branch 'change-has-access-signature' into allow-parameters-on-e…
5ef2031
rename `object` to `obj`
1f16998
support both objects and group dicts in `has_access` and `require_acc…
029c395
simplify permission tests once `has_access` accepts groups
0ff0cc9
Merge branch 'change-has-access-signature' into allow-parameters-on-e…
ff4dfbb
change has_access and require_access signatures to work with the obje…
71452cd
rename `object` to `obj`
371c950
support both objects and group dicts in `has_access` and `require_acc…
90a8eac
simplify permission tests once `has_access` accepts groups
9c6ce65
Merge branch 'change-has-access-signature' into allow-parameters-on-e…
c88d58a
fix bad rebase
818fc27
send embed parameters through POST data
5770888
no need to log `is_api_key`
c860314
Merge branch 'master' into allow-parameters-on-embeds
2c78937
move query fetching by api_key to within the Query model
34404b4
fetch user by adding a get_by_id function on the User model
3d24835
pass parameters as POST data (fixes test failure introduced by switching
66251fd
test the right thing - queries with safe parameters should be embeddable
4a32cdb
introduce cy.clickThrough
304b178
add another Cypress test to make sure unsafe queries cannot be embedded
17636ec
serialize Parameters into query string
16a8a40
set is_api_key as the last parameter to (hopefully) avoid
1e7bd32
Update redash/models/parameterized_query.py
arikfr 2efc9da
Merge branch 'master' into allow-parameters-on-embeds
8f81978
attempt to fix empty percy snapshots
e912010
snap percies after DOM is fully loaded
64f1170
Merge branch 'allow-parameters-on-embeds' of github.com:getredash/red…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
describe('Embedded Queries', () => { | ||
beforeEach(() => { | ||
cy.login(); | ||
cy.visit('/queries/new'); | ||
}); | ||
|
||
it('are shared with safe parameters', () => { | ||
cy.getByTestId('QueryEditor') | ||
.get('.ace_text-input') | ||
.type('SELECT * FROM organizations WHERE id=\'{{}{{}id}}\'{esc}', { force: true }); | ||
|
||
cy.getByTestId('TextParamInput').type('1'); | ||
cy.clickThrough(` | ||
ParameterSettings-id | ||
ParameterTypeSelect | ||
NumberParameterTypeOption | ||
SaveParameterSettings | ||
ExecuteButton | ||
SaveButton | ||
`); | ||
cy.getByTestId('ShowEmbedDialogButton').click({ force: true }); | ||
cy.getByTestId('EmbedIframe').invoke('text').then((iframe) => { | ||
const embedUrl = iframe.match(/"(.*?)"/)[1]; | ||
cy.logout(); | ||
cy.visit(embedUrl); | ||
cy.getByTestId('VisualizationEmbed', { timeout: 10000 }).should('exist'); | ||
cy.percySnapshot('Successfully Embedded Parameterized Query'); | ||
}); | ||
}); | ||
|
||
it('cannot be shared with unsafe parameters', () => { | ||
cy.getByTestId('QueryEditor') | ||
.get('.ace_text-input') | ||
.type('SELECT * FROM organizations WHERE name=\'{{}{{}name}}\'{esc}', { force: true }); | ||
|
||
cy.getByTestId('TextParamInput').type('Redash'); | ||
cy.clickThrough(` | ||
ParameterSettings-name | ||
ParameterTypeSelect | ||
TextParameterTypeOption | ||
SaveParameterSettings | ||
ExecuteButton | ||
SaveButton | ||
`); | ||
cy.getByTestId('ShowEmbedDialogButton').click({ force: true }); | ||
cy.getByTestId('EmbedIframe').invoke('text').then((iframe) => { | ||
const embedUrl = iframe.match(/"(.*?)"/)[1]; | ||
cy.logout(); | ||
cy.visit(embedUrl, { failOnStatusCode: false }); // prevent 403 from failing test | ||
cy.getByTestId('ErrorMessage', { timeout: 10000 }).should('exist'); | ||
cy.percySnapshot('Unsuccessfully Embedded Parameterized Query'); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have code in that takes parameters from the query string, we should reuse it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you help me locate the piece of code you are mentioning? I did find similar behavior when constructing a Parameters instance, but that is also bound to the actual query text, and doesn't seem to have the built-in ability to convert to a simple key-value JSON.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I guess we need to consolidate this. As a quick fix there's this, but we should really have this logic in one place. Could you let me know which piece of code you were mentioning in this comment?