-
Notifications
You must be signed in to change notification settings - Fork 199
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
[Don't review yet] Access hostname for app's #2328
Closed
Closed
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
044498e
Initial
shrshindeMSFT 4ad8d2e
Update .
shrshindeMSFT 0f49f5d
Updatte
shrshindeMSFT 48edc1a
Merge branch 'main' into shrshinde/appRunningHost
shrshindeMSFT 4641091
Update tests
shrshindeMSFT 9f9390b
Merge branch 'main' into shrshinde/appRunningHost
shrshindeMSFT e01fbed
Create CODE_OF_CONDUCT.md (#2331)
TrevorJoelHarris bf11a79
Create SECURITY.md (#2332)
TrevorJoelHarris b429900
Update all link navigation functions to clarify appropriate usage (#2…
AE-MS de0972d
Publish xctest result for screenshots and diagnostic logs (#2306)
KangxuanYe 739ddf3
Added url search parameters for insecure e2e testing (#2336)
shrshindeMSFT 996f776
Update 2.0.0 version references in docs to be TeamsJS v2.0.0 per docs…
AE-MS a6bb4cc
Merge branch 'main' into shrshinde/appRunningHost
shrshindeMSFT 67fd7ce
Changefile
shrshindeMSFT 2405520
Add react router for navigation testing (#2275)
alexneyman-MSFT a019440
Merge branch 'main' into shrshinde/appRunningHost
shrshindeMSFT 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
7 changes: 7 additions & 0 deletions
7
change/@microsoft-teams-js-1ad0d077-794c-4213-aa67-eed6b8c3ec75.json
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,7 @@ | ||
{ | ||
"type": "minor", | ||
"comment": "Added a small capability which returns the host name where the app is hosted.", | ||
"packageName": "@microsoft/teams-js", | ||
"email": "shrshinde@microsoft.com", | ||
"dependentChangeType": "patch" | ||
} |
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
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.
Looking at how this function is called in
app.ts
, it looks in some cases the app won't be initialized yet when you hit this line. Can I call like this actually work before the app is initialized? I think it wouldn't but maybe I'm forgetting/missing something. I think if there's no host to send it to the message will just sit forever and never receive a response.If I assume that this won't work before
initialize
then I think this approach will have some problems.setWindows
will not throw an error right now if the app is hosted in an iframe that isn't the hub-sdk for example.I wonder if a slightly different approach could work. There are two different configurations TeamsJS can be in if the app that is using it is being hosted:
For case 1, I think you could check if that one is the case relatively easily: if the current window is an
ExtendedWindow
that hasnativeInterface
defined on it, we can assume that the app is probably hosted by a host that is using a webview. Theoretically, someone that is not an official host could be running the app in a webview, but if they are it is unlikely that they have set up the WebView this way. See lines 104-107For case 2 it is probably trickier. One thing you might be able to do is look at the domain that
window.top
is running at and then compare that domain to the list of validOrigins from the CDN. We may want to let the app developer pass in an additional set of "valid" host origins to check for in addition to the CDN list (the same list of valid origins they can pass to initialize).If either of the cases above are true, we can tell the developer that they are probably being hosted by a host that will respond to their initialize call. We still might sometimes get it wrong, but in the worst case app developers will still be doing the same thing they are now: calling initialize and waiting for it to timeout.
What do you think?
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.
When the app is not hosted the set windows will throw error
Initialization failed. ...
and we can catch that error and tell the developer that app is not hosted. Inapp.ts
we check ifGlobalVars.isInitializeCompleted
then usesendAndMessageToParentAsync
as this will straight give the host name. For the second part where the app is not initializedsetWindows
sets the windows for communication and if it doesn't find any then it just throws error, if it finds the top window we send the message which will be handled by the hubSDK.