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

Add Microsoft Teams for Linux (Electron) profile #2710

Merged

Conversation

TheDarkTrumpet
Copy link
Contributor

No description provided.

Copy link
Collaborator

@rusty-snake rusty-snake left a comment

Choose a reason for hiding this comment

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

Looks great, but a little nitpicking.

private-dev
private-etc fonts,machine-id,localtime,ld.so.cache,ca-certificates,ssl,pki,crypto-policies,resolv.conf
private-tmp
disable-mnt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move over private-bin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the only comment I wasn't sure about what was meant. When you mention move over private-bin, what do you mean? Thanks again for all your help in looking at this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This order:

disable-mnt
private-bin
private-cache
private-dev
private-etc
private-tmp

etc/teams-for-linux.profile Show resolved Hide resolved
etc/teams-for-linux.profile Outdated Show resolved Hide resolved
etc/teams-for-linux.profile Outdated Show resolved Hide resolved
etc/teams-for-linux.profile Outdated Show resolved Hide resolved
etc/teams-for-linux.profile Outdated Show resolved Hide resolved
etc/teams-for-linux.profile Show resolved Hide resolved
@rusty-snake
Copy link
Collaborator

Also add teams-for-linux to src/firecfg/firecfg.config.

@Fred-Barclay
Copy link
Collaborator

@TheDarkTrumpet any updates on this? 😄

@rusty-snake
Copy link
Collaborator

@Fred-Barclay note his status 😉 : I may be slow to respond.

@TheDarkTrumpet
Copy link
Contributor Author

heh, thanks for the poke on that. Sorry for not responding earlier - starting a new job soon so work has been pretty crazy right now trying to wrap up stuff in current job and prepare for the new one. I have time this weekend to implement the changes and verify. I'll have them pushed by the end of this weekend

@Fred-Barclay
Copy link
Collaborator

@TheDarkTrumpet Thanks! No rush, just was wondering if there was an update. 😃 Congrats on the new job!

@rusty-snake Ah yes, got to remember we have that feature on GitHub now..

@TheDarkTrumpet
Copy link
Contributor Author

Alright, I think I got all the changes added minus the "move over private-bin" part. Please let me know if there's anything I missed or anything you suggest I try adding. I'll probably spend some more time adding new profiles for other programs I use as there's a few that could benefit from this. Thanks for putting together this fantastic project.

@SkewedZeppelin
Copy link
Collaborator

@rusty-snake
Copy link
Collaborator

Uhh I see @SkewedZeppelin has already posted a patch.

@TheDarkTrumpet
Copy link
Contributor Author

I pushed the changes according to the patch (I didn't apply it directly since some was already added).

I'm thinking a bit, and I'm curious if the https://github.com/netblue30/firejail/blob/master/CONTRIBUTING.md can be improved a bit. At first, I could see the improvements from a security standpoint, but quickly became more about making things alphabetical and grouped in certain ways. I definitely understand the desire to keep things clean though. While I feel like I got a decent handle on the desires at this point, it wasn't until the diff popped up that I noticed the alphabetical nature of it all. It probably didn't help I used Discord as a jumping off point for Teams since they're similar in terms of functionality.

I do want to help with other profiles, and I want to see others also help with the profiles. I think updating the Contributing that kinda spells out at least the formatting desires would be great, then the bulk of the pull requests could be better served in adding and tightening security concerns.

@SkewedZeppelin SkewedZeppelin merged commit 8fe4e69 into netblue30:master Jun 4, 2019
@SkewedZeppelin
Copy link
Collaborator

This looks fine now.
Merged, thanks.

As for the format and new contributor issues, they are very real.
But many have been working towards improving it.
See the recently added template
https://github.com/netblue30/firejail/blob/master/etc/templates/profile.template
and the goals on improving documentation and a wiki in #2729

@TheDarkTrumpet
Copy link
Contributor Author

That profile.template is amazingly helpful. Wish I found that earlier.

I found another issue (#2730 ) that I commented on as well. Going to subscribe to both issues. The tl;dr for pretty much anything I can recommend is the Table of Contents, or the jumping off point, should be bubbled up to the most likely place people would start looking. Contrib, or readme. I'll mention the same thing in #2729 to keep issue discussion with issues.

@TheDarkTrumpet TheDarkTrumpet deleted the 1139-AddTeamsProfile branch June 4, 2019 01:24
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