Closed Bug 772192 Opened 12 years ago Closed 11 years ago

Enable Embedding Of Smartsheet Platform Content

Categories

(Infrastructure & Operations Graveyard :: WebOps: Other, task, P1)

Product:
Infrastructure & Operations Graveyard
Component:
WebOps: Other
Platform:
x86
macOS
Type:
task

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: bensternthal, Assigned: nmaul)

References

Details

(Whiteboard: [triaged 20120904]) 

Our group Websites & Developer Engagement are using smartsheet as our go to scheduling application. We would like to have the ability to embed smartsheet tables into the wiki.

Smartsheet provides an html link that can be embedded via an iframe:

http://publish.smartsheet.com/830db6dd115240ddbe1efb7e40464c95

<IFRAME WIDTH=1000 HEIGHT=700 FRAMEBORDER=0 SRC="http://publish.smartsheet.com/830db6dd115240ddbe1efb7e40464c95"></IFRAME>

I can not figure out any way to get either the iframe or just the html link to be embedded in the wiki. We would benefit very much from having this ability. We are open to whatever you suggest to accomplish this (white-listing, special user permissions, etc).
Could iframes be allowed *only* from the *.smartsheet.com domain?
Requesting an update on this.

Please let me know if this is possible.
Assignee: nobody → server-ops-webops
Component: wiki.mozilla.org → Server Operations: Web Operations
Product: Websites → mozilla.org
QA Contact: cshields
Version: unspecified → other
Requesting an update on this. 

Many thanks.
Honestly I have no idea how we could go about doing this. Allowing user-insertable iframes sounds like a big security concern, and since it's probably just content as far as Mediawiki is concerned, it might not be easy to filter.

It might be possible to do something if we can find a suitable Mediawiki extension/plugin that has good security controls around it, but I haven't found anything yet that doesn't come with a massive disclaimer about being insecure or no longer maintained.
Jake:

What if it was something custom like the bugzilla integration (Brandon Savage) where you pass mediawiki a special tag with string that represents the published sheet? That way, all of the interaction and filtering is controled from the plugin and not a whitelist for iframes?

Thanks
Sounds good to me. We'd still want to send it through AppSec for a security review, but I like that idea. :)
For sure! How should we proceed? We don't have appear to have a WikiMo team like we do with BMO or other platforms to do custom work. Anyone specific we can talk to?
I was hoping you had someone in mind already. :)

I would talk to someone in Web Dev about it (maybe start with morgamic), or maybe Brandon Savage would be able to help out with this too.

You could also try to solicit something from Smartsheet... they might be interested to have such a plugin available for customers to use. They might even have something in the works already.
I poked around a bit, this might be pretty easy to write.. I am imagining something like (pseduo code):

<smartsheet>
    sheet_id = "53e9029517dc4d8c8b799eaacfee9a1e"
    width = "500"
    height = "500"
</smartsheet>

Which would generate

<iframe src="http://www.smartsheet.com/53e9029517dc4d8c8b799eaacfee9a1e" width="500" height ="500">

Since we are only getting the sheet id via the wiki, I do not think this would have all the security risks as plugins that allow you to specify the domain in the wiki. 

As long as we made sure sheet_id was sanitized, I do not think folks could mess with calling a url other than smartsheet.

Thoughts?
Assignee: server-ops-webops → bsternthal
Coded, I asked brandon via email to review prior to a sec review. 

https://github.com/bensternthal/Smartsheet-MediaWiki-Extension
+1 to Ben for just doing it! Nice.
Jake:

I have not heard anything from brandon so I would like to move forward with a sec review.

To do this I think I need to have this installed on a dev server somewhere. Do we have a dev server for the mozilla wiki, if so do you know whom I would contact to get the plugin installed to it could be tested?

thanks.
Adding sec-review keyword. Moving forward without dev install as according to Mana "wiki.allizom.org is Generic Cluster on but not currently usable"
Keywords: sec-review-needed
There is also wiki-dev.allizom.org. However both seem to be broken at the moment... they require Auth, and claim to want LDAP auth, but my LDAP credentials don't work. Will take some investigation.
Flags: sec-review?
Keywords: sec-review-needed
Flags: sec-review? → sec-review?(amuntner)
Pinging for an updated eta on this one as I see it was assigned.

many thanks.
Don't have an update for you, but I unassigned this from you... it should be unassigned until someone in webops gets the green light to install it. :)

Side note for comment 14... the stage wiki site works again. Dev is still broken (for other reasons), but we do have some place we could set this up for testing purposes. Appsec, let us know if you want that.
Assignee: bsternthal → server-ops-webops
Whiteboard: [triaged 20120904][waiting][appsec]
This is the kind of thing that makes goals easier to work with, so even though it itself isn't a goal, I'm marking it as P2 (goal-impacting bug).
Priority: -- → P2
Can you provide a completion estimate for this, month or quarter?
Sorry, there's nothing webops can do on this until security weighs in. I couldn't even tell you what the timeline on that is. I'm bumping our priority to P1 (P2 is actually for security-related bugs, not goal bugs).

Adam, any thoughts on this?
Priority: P2 → P1
Ping!
Flags: needinfo?(amuntner)
Hi Jake, I'll start taking a look at this.

Where is it actually up and running, now?
Flags: needinfo?(amuntner)
This is installed on wiki-dev.allizom.org. I've made a page that uses it:

https://wiki-dev.allizom.org/User:Stagetest

... but for some reason I'm getting an 'access denied' XML result from Smartsheet. I suspect I have used bad smartsheet ID tags, and/or those pages are not accessible via this method. Ben might be able to help here.

Ben, I'm trying to access either of:

https://www.smartsheet.com/b/publish?EQBCT=355f3fbc546c441e8bb02955be648bc6
https://www.smartsheet.com/b/publish?EQBCT=453ea4f562ad4b778f0a3b077edc7ddf

Do you need to do anything to make those usable? Is it proper to simply use the EQBCT= value for "id", like this?:

<smartsheet id="453ea4f562ad4b778f0a3b077edc7ddf" width="700" height="400" />
<smartsheet id="355f3fbc546c441e8bb02955be648bc6" width="700" height="400" />

Alternatively, if you have better links/id's for testing purposes, that would be fine too.

Adam: it's only a single 72-line PHP script, so hopefully you won't need to rely on seeing it in action too much. :)
Flags: needinfo?(booboobenny+bugzilla)
Can you provide credentials for me to see the test page?
Flags: needinfo?(booboobenny+bugzilla)
Should just be your LDAP credentials. If you need an actual wiki account once you're past the HTTP basic auth, best bet is just to make one.
Ok here is an id that will work:

53e9029517dc4d8c8b799eaacfee9a1e

To embed a smartsheet they need to have publish settings set (in smartsheet)

I added an example into that test page that works.
Flags: sec-review?(amuntner) → sec-review?(fbraun)
Okay, I took a look at the source code (staging server doesnt work at the moment).

There's a minor thing though, It's not really a security issue but I'd like to mention this anyway:

Input Validation is handled with htmlspecialchars (which is kind of a black list)

While this is really good I'd rather like the script to use typecasts to int for width and height. In case this is of anyone's concern (not too sure here) one could also do sanity checking on the actual integer value. It's a little messy if people choose *really* big values for the iframe size.

Since the smatsheet id always appears to be a alphanumerical value one could have used a regex to check for a-z0-9.


Again, these are no security vulnerabilities. But in my opinion it makes more sense to check for the input you expect (white list approach) than to block everything that might cause a problem (black list approach, i.e. htmlspecialchars).
Ben: could you do a regex for the smartsheet ID instead of the specialchars?
I'll implement the regex for the smartsheet id and probably will check for ints for height and width.

I would prefer to leave out checking the height / width values, folks might want really big embeds, especially if they are using a ghantt chart.

Although personally I would like to prevent folks from ever using a ghantt chart :)

Will try to complete by the end of the week.
OK after poking around I suggest leaving this as is.

Height & Width = Can't guarantee this will be INT, folks may want 100% width or 50% width for example.

Smartsheet ID = Thinking about this I would prefer to only use the html special char and not do a regex for a-z/0-9. It looks like smartsheet now is only a-z/0-9 but I have no idea if this will be the case in the future.
(In reply to Ben (:bensternthal) from comment #29)
> OK after poking around I suggest leaving this as is.
> 
> Height & Width = Can't guarantee this will be INT, folks may want 100% width
> or 50% width for example.
> 
> Smartsheet ID = Thinking about this I would prefer to only use the html
> special char and not do a regex for a-z/0-9. It looks like smartsheet now is
> only a-z/0-9 but I have no idea if this will be the case in the future.

I'm fine with that so we can start using it. IT is begging for this and they asked me two days ago when it will be live and available.
Ok I then based on the above we have a green light to install.

Jake do we need to do anything additional here? Is this bug sufficient for the actual install task?
Flags: needinfo?(nmaul)
Just wanted to ping on this, I think we are done and just need it installed :)
I know that Melissa O'Connor has been begging for this for a while. She will also be using it immediately.
I'm sorry for the delay... nothing more is needed, I will deploy this today. Very happy to get this into production!
Flags: needinfo?(nmaul)
This is deployed to prod! It worked for me on stage... Ben / Chris, let me know if you have any trouble with this.

Thanks everyone!
Assignee: server-ops-webops → nmaul
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [triaged 20120904][waiting][appsec] → [triaged 20120904]
That works fabulous! Thanks a ton for getting this extension written and published. It enhances the user experience a lot. Here our example for verification:

https://wiki.mozilla.org/Auto-tools/Automation_Development/Projects#Prioritized_Projects
Status: RESOLVED → VERIFIED
Confirmed this is working. Thanks guys for all the help!
Flags: sec-review?(fbraun) → sec-review+
Depends on: 875231
Component: Server Operations: Web Operations → WebOps: Other
Product: mozilla.org → Infrastructure & Operations
Product: Infrastructure & Operations → Infrastructure & Operations Graveyard
You need to log in before you can comment on or make changes to this bug.